-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Summary page improvements #546
Conversation
…essary calls to the server
… is smaller than 400 px, to optimize the display on smaller landscape screens
…ing on the Search link from GET-constructed results page
…ist alert content filter across page reloads
… web workers. Controlled through LOCALAPI parameter in config file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks -- there are many super improvements! I leave here my feedback on suggested changes.
dmc.Paper without options
You systematically removed the options radius='xl', p='md', shadow='xl', withBorder=True
to dmc.Paper
component for lightcurves. I prefer by far to keep the options as before, as rendering without the options is not great to my opinion.
Summary page accordion (right side)
I noticed an opened item does not close anymore when I open a new one. Is it on purpose? I cannot find the option on the code that does that (and I prefer when only one item at a time is open -- i.e. the previous behaviour).
Stamp modal removed
Some people reported they were using the enlarged stamps from the modal because the ones on the right were too small. I will be against making the current cutouts bigger for space reasons, but probably we should reintroduce the modal with enlarged stamps?
Yes, I understand the wish to have them bigger - but in its current form the popup is ugly and useless. Until we get some better idea what to add there - I suppose the zoom feature of smaller cutouts is enough for inspecting the fine details. |
Having several panels open there is essential as otherwise you cannot simultaneously see the cutouts and alert contents for selected point - and it is important when clicking through the data. Actually, I thought you intentionally used |
I propose that until we get some better idea, we keep the previous button. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks good to me -- can I merge these changes?
Hi @JulienPeloton, yes, sure! You may merge it |
a.k.a. New Year PR
Set of visual fixes, adjustments and improvements for more consistent experience on the summary page and around it.
Based on #541 and thus has unnecessarily long commit list shown. Hope it will be reduced after #541 merge.
As usual, demo version is available at http://157.136.250.93:24000