chore: Refactor dashboard header to func component #31029
Conversation
| dashboardInfo.common?.conf?.DASHBOARD_AUTO_REFRESH_MODE === 'fetch' | ||
| ) { | ||
| // force-refresh while auto-refresh in dashboard | ||
| return boundActionCreators.fetchCharts( |
There was a problem hiding this comment.
nit: I know it was like this before this PR, but we could DRY it with a helper function so you can pass the boolean as arg and return the fetchCharts call with it here and the one below
| t( | ||
| 'Your dashboard is too large. Please reduce its size before saving it.', | ||
| ), | ||
| ); | ||
| } else { | ||
| if (positionJSONLength >= limit * 0.9) { | ||
| this.props.addWarningToast('Your dashboard is near the size limit.'); | ||
| boundActionCreators.addWarningToast( | ||
| 'Your dashboard is near the size limit.', |
There was a problem hiding this comment.
Should we translate this?
|
|
||
| if (interval) { | ||
| const periodicRefreshOptions = | ||
| dashboardInfo.common?.conf?.DASHBOARD_AUTO_REFRESH_INTERVALS; |
There was a problem hiding this comment.
nit: Is this value dynamically changing somewhere or just a static config loaded once? If static, we could move this out the callback.
There was a problem hiding this comment.
it doesn't change, but technically it could since it's a part of the redux state object, so it should be included in the dependency array - it won't hurt and it won't trigger a lint warning
|
Maybe it would be nice to use |
good point 🤦 will keep that in mind next time |
SUMMARY
Refactor the dashboard Header component from class to function component. No functional changes, just a refactor
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
Everything should work like before
ADDITIONAL INFORMATION