[FEATURE] Add disable_auto_refresh config - #212
Conversation
…a server config (default false) Signed-off-by: Iris Ye <iris.ye.cs@gmail.com>
2e9c4cb to
e7d4d0f
Compare
|
@mixturesolutions mind checking the tests and lint failures? |
Co-authored-by: Gabriel Bernal <gabrielbernalp@gmail.com> Signed-off-by: Iris Ye <143642859+mixturesolutions@users.noreply.github.com>
Signed-off-by: Iris Ye <iris.ye.cs@gmail.com>
ibakshay
left a comment
There was a problem hiding this comment.
Great Job! I left a minor comment. I also tested manually. Everything is working as expected! 🚀
| if (isAutoRefreshDisabled) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
This check is redundant. The picker is disabled when isAutoRefreshDisabled is true
(line 239), so onChange never fires and this handler is never called. So, this check is safe to be removed.
Signed-off-by: Iris Ye <iris.ye.cs@gmail.com>
|
I was wondering if it is not better to "hide" the selector, instead of disabled effect? Maybe more a question for people that want this feature 😄 |
@ibakshay Since you suggested the feature, what do you think? |
Yes, hiding is fine with me. :) @mixturesolutions |
c366771 to
d2ad478
Compare
| ); | ||
| const refreshIntervalPicker = screen.getByLabelText(/Select refresh interval/i); | ||
| expect(refreshIntervalPicker).not.toHaveAttribute('aria-disabled', 'true'); | ||
| expect(screen.queryByText('Off')).not.toBeInTheDocument(); |
There was a problem hiding this comment.
Not sure this makes sense, are we saying we hide the off option. So users cannot turn off the auto refresh? IIUC the goal is to hide the whole select component as users are not allowed to select any value. cc @ibakshay @mixturesolutions
There was a problem hiding this comment.
Just to clarify, the changes hides the entire refresh interval dropdown when disabled is true. It's just in the case of my ui display during testing, the picker defaults to off option, so the comment was highlighting the "Off" as representing the disabled state of the entire picker. I changed the PR to make the comment less ambiguous. Please reference the description UI photos if it matches what you had in mind.
There was a problem hiding this comment.
Thnx for the update, this test was getting the refresh interval picker and asserting content on it, but the suggestion was to remove the picker entirely as is updated now.
There was a problem hiding this comment.
I saw the last change reverted, maybe we need to check again as this test is asserting something different than the proposal. The test should check that the interval picker does not exist in the DOM.
There was a problem hiding this comment.
I was fixing the commit message for DCO and reverted my commits a couple times and repushed, it might have gotten the git commits out of sync with my current testing environment setup. It was my bad. Everything should be good now.
96e91cb to
fab9473
Compare
…nabled Signed-off-by: Iris Ye <iris.ye.cs@gmail.com>
fab9473 to
920c5a9
Compare
Signed-off-by: Iris Ye <143642859+mixturesolutions@users.noreply.github.com>
| value?: DurationString; | ||
| onChange: (value: DurationString) => void; | ||
| height?: string; | ||
| disabled?: boolean; |
There was a problem hiding this comment.
This is not needed as is unused, the whole component is not rendered
087bfe3 to
dad1325
Compare
…om previous disabling feature Signed-off-by: Iris Ye <iris.ye.cs@gmail.com>
b93c6b6 to
10a5a99
Compare
Description
Adds an optional admin config flag to centrally disable dashboard auto-refresh, addressing perses/perses#4295
Default: false (no behavior change unless explicitly enabled)
When enabled:
Note this is linked with another PR in the main perses repo
Screenshots
To enable, change in the respective perses config yaml file like the following:

Once the changes are deployed, config page will reflect the changes and the dashboard ui will look like:

Originally looks like:

Checklist
[<catalog_entry>] <commit message>naming convention using one of thefollowing
catalog_entryvalues:FEATURE,ENHANCEMENT,BUGFIX,BREAKINGCHANGE,DOC,IGNORE.UI Changes
See e2e docs for more details. Common issues include: