Skip to content

Improve AlertViewer UI/UX #1939

Merged
kpuriIpac merged 1 commit into
devfrom
alert-viewer-changes
Apr 21, 2026
Merged

Improve AlertViewer UI/UX #1939
kpuriIpac merged 1 commit into
devfrom
alert-viewer-changes

Conversation

@kpuriIpac
Copy link
Copy Markdown
Contributor

@kpuriIpac kpuriIpac commented Apr 15, 2026

SUIT PR: lsst/suit#84

  • Remove Job Monitor tab for alert viewer
  • Default tab when you hit /alertviewer is now the Alert tab (/alertviewer/upload) instead of the Results tab.
  • Simplified Results (landing page) text for AlertViewer
    • required a small custom component to modify the bottom section of the LandingPage, I could have probably done this through slotProps, but it was proving to be trickier (was affecting Euclid/Spherex) and LandingPage is used in several places so I thought it was best to create a simple custom component instead in this case
    • And the changes in RoutedApp are to support being able to call LandingPage as well (to have more control over customizing it)

Testing:

@kpuriIpac kpuriIpac self-assigned this Apr 15, 2026
@kpuriIpac kpuriIpac requested review from loitly and robyww April 15, 2026 23:44
Copy link
Copy Markdown
Contributor

@loitly loitly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please revert customLandingPage changes if my suggested solution works. Thanks.

Comment on lines -80 to -42
landingPage: <AlertViewerLanding/>,
slotProps: {
drawer: { drawerWidth:'24rem'},
landing: {
desc: 'Some information about the alert viewer here. This is the landing page. This text will be changed to something more useful in the future.',
bgImage: null,
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t think this is necessary. slotProps.landing was created specifically for this use case. If you need a custom component, you can define it like this:

landing: {
    component: <AlertViewerLanding/>
}

Comment on lines +96 to +99
const hasUrlApi = search.includes('api=');
if (!hasUrlApi && (path === basename || path === `${basename}/`)) {
dispatchOnAppReady(() => dispatchShowDropDown({view: 'AlertUpload'}));
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this logic correct? Normally, ?api would show the api help page, but I am not seeing that happen.
Also, using URLSearchParams might help when working with URL parameters.

const { pathname, search } = window.location;
const params = new URLSearchParams(search);
params.has('api');

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes good suggestion, thank you. also fixes the ?api bug, it should now show the api help page (after I push my commit).

The if check here was needed because we want to re-route user to /alertviewer/upload path when they try to access /alertviewer, but if I didn't add the check for !hasUrlApi, it would also re-route you to /upload path for the URL API instead of taking you directly to the results (when, for instance, you try to access somethign like: /alertviewer?api=alert&id=170059278837088375).

open to better suggestions for this re-route logic though.

@kpuriIpac kpuriIpac force-pushed the alert-viewer-changes branch from 8572512 to 0c46100 Compare April 21, 2026 19:02
Copy link
Copy Markdown
Contributor

@robyww robyww left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UI changes look good. Go ahead and merge when @loitly approves.

Copy link
Copy Markdown
Contributor

@loitly loitly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks for the changes.

@kpuriIpac kpuriIpac force-pushed the alert-viewer-changes branch from 0c46100 to b67ed16 Compare April 21, 2026 20:04
@kpuriIpac kpuriIpac merged commit e22c0af into dev Apr 21, 2026
@kpuriIpac kpuriIpac deleted the alert-viewer-changes branch April 21, 2026 20:05
@robyww robyww added this to the 2026.1 milestone Apr 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants