Add default auto refresh config #21351
Conversation
| import type { RefreshConfig } from 'views/components/contexts/AutoRefreshContext'; | ||
| import { durationToMS } from 'util/DateTime'; | ||
|
|
||
| const defaultInterval = 'PT30S'; |
There was a problem hiding this comment.
I am not sure if we should hardcode it. What are arguments against reusing the search refresh defaults for this? 5s does not seem to be too low for me.
There was a problem hiding this comment.
@dennisoelkers 30 sec that was a product requirement.
There was a problem hiding this comment.
Can you clarify if it was even considered to reuse the configurable values or if that number was just an educated guess, without knowing that a user can configure it in other places?
There was a problem hiding this comment.
I addressed this question. Let's wait for a response
There was a problem hiding this comment.
@dennisoelkers After discussions we decided to use the default interval from configuration. Pushed the changes
| @@ -49,6 +49,12 @@ const AutoRefreshProvider = ({ children, onRefresh }: React.PropsWithChildren<{ | |||
| }; | |||
| }, [refreshConfig?.enabled, refreshConfig?.interval, onRefresh, animationId]); | |||
|
|
|||
| useEffect(() => { | |||
There was a problem hiding this comment.
Instead of this second useEffect, we should set defaultRefreshConfig as the initial value for refreshConfig in the useState call.
* Add default auto refresh config * Add changelog * fix eslint * Add default interval usage * improvement. remove useEffect
Description
This PR adds the option to add default auto-refresh config to
AutoRefreshProviderWe are changing the default config for Events. Now it's starts automatically with a default interval from configuration
Motivation and Context
fix: #21350
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: