superset
superset copied to clipboard
feat: custom refresh frequency
Earlier at dashboard level we had very limited options in 'Set auto-refresh interval' as shown in image below
But as per our organisation level we wanted the frequency to be something else that was not in dropdown, so we implemented that.
As part of this PR, we can add custom frequency as required by user, user can select any frequency in hour, minute and seconds and set it as auto refresh interval frequency (in seconds).
@rusackas @justinpark @mistercrunch @villebro @geido @eschutho @betodealmeida @nytai @craig-rueda @kgabryje @dpgaspar
Please review.
@kasiazjc would you mind looking at the design? Personally I think that custom (and how it is defined/configured) should be a secondary option (hidden by default)—possibly under a collapsed menu, button, tab, etc.
From the screenshots, it looks a little confusing to show the dropdown alongside the text fields. Perhaps a new "virtual" option (CUSTOM
) should be added, which would then hide/show the text boxes when selected? Also, having two buttons when choosing a custom refresh interval looks a little confusing as well.
Codecov Report
Attention: Patch coverage is 40.00000%
with 24 lines
in your changes are missing coverage. Please review.
Project coverage is 67.41%. Comparing base (
9ced255
) to head (4bd1582
). Report is 54 commits behind head on master.
Files | Patch % | Lines |
---|---|---|
.../src/dashboard/components/RefreshIntervalModal.tsx | 40.00% | 23 Missing and 1 partial :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## master #24449 +/- ##
==========================================
+ Coverage 67.34% 67.41% +0.07%
==========================================
Files 1909 1910 +1
Lines 74623 74741 +118
Branches 8324 8356 +32
==========================================
+ Hits 50256 50388 +132
+ Misses 22314 22300 -14
Partials 2053 2053
Flag | Coverage Δ | |
---|---|---|
javascript | 57.37% <40.00%> (+0.16%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Okay sure @john-bodley @craig-rueda I will try to hide custom view and show custom view when required.
I have made changes as asked When user will click CUSTOM button then box changes so that one can provide custom frequency, and if user clicks CUSTOM button again then original window will appear again.
@john-bodley @craig-rueda Please check and review.
I have shown only one refresh change option at a time as requested by @john-bodley and removed extra submit button as requested by @craig-rueda
Sorry for the churn and delays on this, but I think the main issue is UI design, not the technical implementation.
My own opinion: • It would be nice if it had the Select menu (including the Custom option, rather than a button for Custom) • If (and only if) the Custom option is selected from the Select menu, would the additional fields appear (or disappear) • The Select menu would remain present... if you select something other than Custom, the form elements are removed, and the values ignored.
As a side note, I feel like the Submit button should be on the right, and the cancel button on the left. I'm not sure if we have a solid documented pattern around this, though (cc. @kasiazjc)
@Abhishek-kumar-samsung is this frequency only for a session or persistent?
Sorry for the churn and delays on this, but I think the main issue is UI design, not the technical implementation.
My own opinion: • It would be nice if it had the Select menu (including the Custom option, rather than a button for Custom) • If (and only if) the Custom option is selected from the Select menu, would the additional fields appear (or disappear) • The Select menu would remain present... if you select something other than Custom, the form elements are removed, and the values ignored.
As a side note, I feel like the Submit button should be on the right, and the cancel button on the left. I'm not sure if we have a solid documented pattern around this, though (cc. @kasiazjc)
Okay i will do some modification with UI.
@Abhishek-kumar-samsung is this frequency only for a session or persistent?
It is persistent.
@Abhishek-kumar-samsung my dashboard frequency is saving only for a particular session , how can i make it persistent?
Sorry for the churn and delays on this, but I think the main issue is UI design, not the technical implementation. My own opinion: • It would be nice if it had the Select menu (including the Custom option, rather than a button for Custom) • If (and only if) the Custom option is selected from the Select menu, would the additional fields appear (or disappear) • The Select menu would remain present... if you select something other than Custom, the form elements are removed, and the values ignored. As a side note, I feel like the Submit button should be on the right, and the cancel button on the left. I'm not sure if we have a solid documented pattern around this, though (cc. @kasiazjc)
Okay i will do some modification with UI.
Thanks and sorry for the long wait! What I'm thinking in terms of UI improvements is this:
-> instead of button/switcher use single select, for selecting the type of interval, the options are:
-
don’t refresh
-
custom interval (triggers the additional fields as shown below)
-
all of the preset options that are available today (in the example dropdown I listed only two, but should be all of them)
-> set fixed height for the modal to accommodate for disappearing custom fields, so that the height does not jump
I think this way setting up the custom interval will be more intuitive, as we are using common patterns. What do you think and could you possibly take this up @Abhishek-kumar-samsung ?
@kasiazjc
Yes i will take this, i will try to make as you have suggested. I was busy in last few days, so i will work on this from now.
@kasiazjc
Yes i will take this, i will try to make as you have suggested. I was busy in last few days, so i will work on this from now.
Great, thank you! If you need some more design specs let me know. I was using default font sizes and components that we use in Superset :)
@Abhishek-kumar-samsung my dashboard frequency is saving only for a particular session , how can i make it persistent?
@Shisir99 i am sorey, i was not sure that time, yes it is not persistent for me too, that is for a particular session only.
@rusackas isn't that a problem? Automatic refresh interval for just a single session does not makes much sense(according to me, or maybe i missed something), it should be persistent.
@kasiazjc i implemented the design as you have suggested, Please review.
Hmm... the persistence is an interesting fork in the road! I'd suggest keeping the functionality as-is for this PR (session-based) and maybe we can discuss a follow-up feature on a separate thread to persist the settings as part of the dashboard itself (probably in the Dashboard metadata). I assume the session-based setting could/would override the dashboard-level setting, too. This might warrant a separate little design discussion.
@rusackas @justinpark @mistercrunch @villebro @geido @eschutho @betodealmeida @nytai @craig-rueda @kgabryje @dpgaspar @kasiazjc @john-bodley
Please review.
![]()
![]()
@kasiazjc i implemented the design as you have suggested, Please review.
Thank you so much! The only thing I would suggest is changing minutes + seconds to a searchable select dropdown component, so that the numbers for seconds (59) and minutes (59) are fixed. Could be a weird experience if people would type in for example 1 hour, 72 minutes, 342 seconds, so I think to avoid that selects would help.
Hi @kasiazjc I added dropdowns in minute and seconds as you have suggested. Instead of select i used input type with datalist. In datalist i have provided options from 0-59, if user puts some other value and submits then it will display error message.
datalist is looking little different depending on browsers, some previews are as below.
edge browser preview:
firefox browser preview:
chrome browser preview:
I was having problem taking screenshot with dropdown, so i took screenshot using phone.
Please check and review @kasiazjc @rusackas
@kasiazjc @rusackas can you pls check the PR, there's been a long time since no comments, so i thought to remind.
This would be a welcome addition, thanks for the effort.
/testenv up
@yousoph Ephemeral environment spinning up at http://52.12.12.69:8080. Credentials are admin
/admin
. Please allow several minutes for bootstrapping and startup.
Hi @kasiazjc I added dropdowns in minute and seconds as you have suggested. Instead of select i used input type with datalist. In datalist i have provided options from 0-59, if user puts some other value and submits then it will display error message.
datalist is looking little different depending on browsers, some previews are as below.
edge browser preview:
firefox browser preview:
chrome browser preview:
I was having problem taking screenshot with dropdown, so i took screenshot using phone.
Please check and review @kasiazjc @rusackas
Sorry for the super late review! Missed this PR. These are not ant design components right? I think single select with predefined list in the dropdown we have in code should be the one used in this for consistency (design and patterns). You can find it in filters for example, and users can also type in some numbers (it just would not accept the new number)
Sorry for the super late review! Missed this PR. These are not ant design components right? I think single select with predefined list in the dropdown we have in code should be the one used in this for consistency (design and patterns). You can find it in filters for example, and users can also type in some numbers (it just would not accept the new number)
@kasiazjc I have made changes as per your suggestion, i have removed input and put select dropdown, now one can also filter based on select dropdown options, now it looks exactly similar to earlier dropdowns, i have checked in chrome, edge, firefox, it is similar in all browsers.
/testenv up
@yousoph Ephemeral environment spinning up at http://54.212.14.85:8080. Credentials are admin
/admin
. Please allow several minutes for bootstrapping and startup.