superset icon indicating copy to clipboard operation
superset copied to clipboard

feat: custom refresh frequency

Open Abhishek-kumar-samsung opened this issue 1 year ago • 59 comments

Earlier at dashboard level we had very limited options in 'Set auto-refresh interval' as shown in image below

107 99 45 216_9000_superset_dashboard_5__native_filters_key=6ZOG4aE3VjGvXnNg9wOkgTrlcVVgk1AgOh1wN3MlrGFIXarapEImbmrarIlIKHQq (3)

normal

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).

107 99 45 216_9000_superset_dashboard_5__native_filters_key=6ZOG4aE3VjGvXnNg9wOkgTrlcVVgk1AgOh1wN3MlrGFIXarapEImbmrarIlIKHQq

107 99 45 216_9000_superset_dashboard_5__native_filters_key=6ZOG4aE3VjGvXnNg9wOkgTrlcVVgk1AgOh1wN3MlrGFIXarapEImbmrarIlIKHQq (2)

107 99 45 216_9000_superset_dashboard_5__native_filters_key=6ZOG4aE3VjGvXnNg9wOkgTrlcVVgk1AgOh1wN3MlrGFIXarapEImbmrarIlIKHQq (1)

Abhishek-kumar-samsung avatar Jun 19 '23 17:06 Abhishek-kumar-samsung

@rusackas @justinpark @mistercrunch @villebro @geido @eschutho @betodealmeida @nytai @craig-rueda @kgabryje @dpgaspar

Please review.

Abhishek-kumar-samsung avatar Jun 19 '23 17:06 Abhishek-kumar-samsung

@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.

john-bodley avatar Jun 20 '23 17:06 john-bodley

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.

craig-rueda avatar Jun 20 '23 17:06 craig-rueda

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.

codecov[bot] avatar Jun 20 '23 17:06 codecov[bot]

Okay sure @john-bodley @craig-rueda I will try to hide custom view and show custom view when required.

Abhishek-kumar-samsung avatar Jun 20 '23 18:06 Abhishek-kumar-samsung

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.

107 99 45 216_9000_superset_dashboard_5__native_filters_key=91ksfxXf42UqpEDLp_k4NNvRNLAqehP-I5U_tTUR-qOzjqVLz4Xr-SWMRYh8cGHh (1)

107 99 45 216_9000_superset_dashboard_5__native_filters_key=91ksfxXf42UqpEDLp_k4NNvRNLAqehP-I5U_tTUR-qOzjqVLz4Xr-SWMRYh8cGHh

Abhishek-kumar-samsung avatar Jun 21 '23 17:06 Abhishek-kumar-samsung

@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

Abhishek-kumar-samsung avatar Jun 21 '23 17:06 Abhishek-kumar-samsung

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)

rusackas avatar Jul 10 '23 14:07 rusackas

@Abhishek-kumar-samsung is this frequency only for a session or persistent?

Shisir99 avatar Jul 19 '23 11:07 Shisir99

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 avatar Jul 19 '23 11:07 Abhishek-kumar-samsung

@Abhishek-kumar-samsung is this frequency only for a session or persistent?

It is persistent.

Abhishek-kumar-samsung avatar Jul 19 '23 11:07 Abhishek-kumar-samsung

@Abhishek-kumar-samsung my dashboard frequency is saving only for a particular session , how can i make it persistent?

Shisir99 avatar Jul 20 '23 06:07 Shisir99

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

image

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 avatar Jul 27 '23 14:07 kasiazjc

@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.

Abhishek-kumar-samsung avatar Jul 27 '23 18:07 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.

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 :)

kasiazjc avatar Jul 28 '23 15:07 kasiazjc

@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.

Abhishek-kumar-samsung avatar Aug 03 '23 17:08 Abhishek-kumar-samsung

dont_refresh custom 10_sec

@kasiazjc i implemented the design as you have suggested, Please review.

Abhishek-kumar-samsung avatar Aug 03 '23 17:08 Abhishek-kumar-samsung

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 avatar Aug 03 '23 18:08 rusackas

@rusackas @justinpark @mistercrunch @villebro @geido @eschutho @betodealmeida @nytai @craig-rueda @kgabryje @dpgaspar @kasiazjc @john-bodley

Please review.

Abhishek-kumar-samsung avatar Aug 04 '23 18:08 Abhishek-kumar-samsung

dont_refresh custom 10_sec

@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.

kasiazjc avatar Aug 07 '23 14:08 kasiazjc

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: IMG_0831

firefox browser preview: IMG_0830

chrome browser preview: IMG_0829

I was having problem taking screenshot with dropdown, so i took screenshot using phone.

Please check and review @kasiazjc @rusackas

Abhishek-kumar-samsung avatar Aug 15 '23 18:08 Abhishek-kumar-samsung

@kasiazjc @rusackas can you pls check the PR, there's been a long time since no comments, so i thought to remind.

Abhishek-kumar-samsung avatar Sep 04 '23 07:09 Abhishek-kumar-samsung

This would be a welcome addition, thanks for the effort.

LyleScott avatar Oct 04 '23 06:10 LyleScott

/testenv up

yousoph avatar Oct 10 '23 22:10 yousoph

@yousoph Ephemeral environment spinning up at http://52.12.12.69:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

github-actions[bot] avatar Oct 10 '23 22:10 github-actions[bot]

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: IMG_0831

firefox browser preview: IMG_0830

chrome browser preview: IMG_0829

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)

kasiazjc avatar Oct 11 '23 11:10 kasiazjc

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.

Abhishek-kumar-samsung avatar Oct 14 '23 16:10 Abhishek-kumar-samsung

IMG_1487

Abhishek-kumar-samsung avatar Oct 14 '23 16:10 Abhishek-kumar-samsung

/testenv up

yousoph avatar Oct 16 '23 21:10 yousoph

@yousoph Ephemeral environment spinning up at http://54.212.14.85:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

github-actions[bot] avatar Oct 16 '23 21:10 github-actions[bot]