AppChrome: Fix race condition when updating chrome state on route changed (#52694)

This commit is contained in:
Torkel Ödegaard
2022-07-29 17:16:14 +02:00
committed by GitHub
parent b8e4c2abeb
commit d3323f870e
2 changed files with 25 additions and 13 deletions
@@ -8,7 +8,7 @@ import { isShallowEqual } from 'app/core/utils/isShallowEqual';
import { RouteDescriptor } from '../../navigation/types'; import { RouteDescriptor } from '../../navigation/types';
export interface AppChromeState { export interface AppChromeState {
chromeless: boolean; chromeless?: boolean;
sectionNav: NavModelItem; sectionNav: NavModelItem;
pageNav?: NavModelItem; pageNav?: NavModelItem;
actions?: React.ReactNode; actions?: React.ReactNode;
@@ -20,6 +20,8 @@ const defaultSection: NavModelItem = { text: 'Grafana' };
export class AppChromeService { export class AppChromeService {
searchBarStorageKey = 'SearchBar_Hidden'; searchBarStorageKey = 'SearchBar_Hidden';
private currentRoute?: RouteDescriptor;
private routeChangeHandled?: boolean;
readonly state = new BehaviorSubject<AppChromeState>({ readonly state = new BehaviorSubject<AppChromeState>({
chromeless: true, // start out hidden to not flash it on pages without chrome chromeless: true, // start out hidden to not flash it on pages without chrome
@@ -27,22 +29,30 @@ export class AppChromeService {
searchBarHidden: store.getBool(this.searchBarStorageKey, false), searchBarHidden: store.getBool(this.searchBarStorageKey, false),
}); });
routeMounted(route: RouteDescriptor) { registerRouteRender(route: RouteDescriptor) {
this.update({ if (this.currentRoute !== route) {
chromeless: route.chromeless === true, this.currentRoute = route;
sectionNav: defaultSection, this.routeChangeHandled = false;
pageNav: undefined, }
actions: undefined,
});
} }
update(state: Partial<AppChromeState>) { update(update: Partial<AppChromeState>) {
const current = this.state.getValue(); const current = this.state.getValue();
const newState: AppChromeState = { const newState: AppChromeState = {
...current, ...current,
...state,
}; };
// when route change update props from route and clear fields
if (!this.routeChangeHandled) {
newState.actions = undefined;
newState.pageNav = undefined;
newState.sectionNav = defaultSection;
newState.chromeless = this.currentRoute?.chromeless;
this.routeChangeHandled = true;
}
Object.assign(newState, update);
if (!isShallowEqual(current, newState)) { if (!isShallowEqual(current, newState)) {
this.state.next(newState); this.state.next(newState);
} }
+5 -3
View File
@@ -14,9 +14,9 @@ export interface Props extends Omit<GrafanaRouteComponentProps, 'queryParams'> {
export function GrafanaRoute(props: Props) { export function GrafanaRoute(props: Props) {
const { chrome } = useGrafana(); const { chrome } = useGrafana();
useEffect(() => { chrome.registerRouteRender(props.route);
chrome.routeMounted(props.route);
useEffect(() => {
updateBodyClassNames(props.route); updateBodyClassNames(props.route);
cleanupDOM(); cleanupDOM();
reportPageview(); reportPageview();
@@ -26,7 +26,9 @@ export function GrafanaRoute(props: Props) {
navigationLogger('GrafanaRoute', false, 'Unmounted', props.route); navigationLogger('GrafanaRoute', false, 'Unmounted', props.route);
updateBodyClassNames(props.route, true); updateBodyClassNames(props.route, true);
}; };
}, [chrome, props.route, props.match]); // props.match instance change even though only query params changed so to make this effect only trigger on route mount we have to disable exhaustive-deps
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
useEffect(() => { useEffect(() => {
// unbinds all and re-bind global keybindins // unbinds all and re-bind global keybindins