Skip to content

Commit

Permalink
Merge pull request #12979 from richard-cox/fix-sidenav-flicker
Browse files Browse the repository at this point in the history
Two side nav improvements
  • Loading branch information
richard-cox authored Jan 6, 2025
2 parents bb18b75 + 8dcc722 commit 4647776
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 11 deletions.
14 changes: 14 additions & 0 deletions shell/components/nav/TopLevelMenu.helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,12 @@ export abstract class BaseTopLevelMenuHelper {
this.$store = $store;

this.hasProvCluster = this.$store.getters[`management/schemaFor`](CAPI.RANCHER_CLUSTER);

// Reduce flicker when component is recreated on a different layout
const { clustersPinned = [], clustersOthers = [] } = this.$store.getters['sideNavCache'] || {};

this.clustersPinned.push(...clustersPinned);
this.clustersOthers.push(...clustersOthers);
}

protected convertToCluster(mgmtCluster: MgmtCluster, provCluster: ProvCluster): TopLevelMenuCluster {
Expand All @@ -145,6 +151,10 @@ export abstract class BaseTopLevelMenuHelper {
clusterRoute: { name: 'c-cluster-explorer', params: { cluster: mgmtCluster.id } }
};
}

protected cacheClusters() {
this.$store.dispatch('setSideNavCache', { clustersPinned: this.clustersPinned, clustersOthers: this.clustersOthers });
}
}

/**
Expand Down Expand Up @@ -258,6 +268,8 @@ export class TopLevelMenuHelperPagination extends BaseTopLevelMenuHelper impleme

this.clustersPinned.push(..._clustersPinned);
this.clustersOthers.push(..._clustersNotPinned);

this.cacheClusters();
}

private constructParams({
Expand Down Expand Up @@ -401,6 +413,8 @@ export class TopLevelMenuHelperLegacy extends BaseTopLevelMenuHelper implements

this.clustersPinned.push(..._clustersPinned);
this.clustersOthers.push(..._clustersNotPinned);

this.cacheClusters();
}

/**
Expand Down
45 changes: 35 additions & 10 deletions shell/components/nav/TopLevelMenu.vue
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,17 @@ export default {
const provClusters = !canPagination && hasProvCluster ? this.$store.getters[`management/all`](CAPI.RANCHER_CLUSTER) : [];
const mgmtClusters = !canPagination ? this.$store.getters[`management/all`](MANAGEMENT.CLUSTER) : [];
if (!canPagination) {
// Reduce the impact of the initial load, but only if we're not making a request
const args = {
pinnedIds: this.pinnedIds,
searchTerm: this.search,
unPinnedMax: this.maxClustersToShow
};
helper.update(args);
}
return {
shown: false,
displayVersion,
Expand All @@ -54,8 +65,9 @@ export default {
canPagination,
helper,
debouncedHelperUpdateSlow: debounce((...args) => this.helper.update(...args), 750),
debouncedHelperUpdateQuick: debounce((...args) => this.helper.update(...args), 200),
debouncedHelperUpdateSlow: debounce((...args) => this.helper.update(...args), 1000),
debouncedHelperUpdateMedium: debounce((...args) => this.helper.update(...args), 750),
debouncedHelperUpdateQuick: debounce((...args) => this.helper.update(...args), 200),
provClusters,
mgmtClusters,
};
Expand Down Expand Up @@ -264,34 +276,44 @@ export default {
this.shown = false;
},
// Before SSP world all of these changes were kicked off given Vue change detection to properties in a computed method.
// Changes could come from two scenarios
// 1. Changes made by the user (pin / search). Could be tens per second
// 2. Changes made by rancher to clusters (state, label, etc change). Could be hundreds a second
// They can be restricted to help the churn caused from above
// 1. When SSP enabled reduce http spam
// 2. When SSP is disabled (legacy) reduce fn churn (this was a known performance customer issue)
pinnedIds: {
immediate: true,
handler(neu, old) {
if (sameContents(neu, old)) {
return;
}
// Low throughput (user click). Changes should be shown quickly
this.updateClusters(neu, 'quick');
}
},
search() {
this.updateClusters(this.pinnedIds, 'slow');
// Medium throughput. Changes should be shown quickly, unless we want to reduce http spam in SSP world
this.updateClusters(this.pinnedIds, this.canPagination ? 'medium' : 'quick');
},
provClusters: {
handler() {
// Shouldn't get here if SSP
this.updateClusters(this.pinnedIds, 'slow');
handler(neu, old) {
// Potentially incredibly high throughput. Changes should be at least limited (slow if state change, quick if added/removed). Shouldn't get here if SSP
this.updateClusters(this.pinnedIds, neu?.length === old?.length ? 'slow' : 'quick');
},
deep: true,
immediate: true,
},
mgmtClusters: {
handler() {
// Shouldn't get here if SSP
this.updateClusters(this.pinnedIds, 'slow');
handler(neu, old) {
// Potentially incredibly high throughput. Changes should be at least limited (slow if state change, quick if added/removed). Shouldn't get here if SSP
this.updateClusters(this.pinnedIds, neu?.length === old?.length ? 'slow' : 'quick');
},
deep: true,
immediate: true,
Expand Down Expand Up @@ -424,7 +446,7 @@ export default {
};
},
updateClusters(pinnedIds, speed = 'slow' | 'quick') {
updateClusters(pinnedIds, speed = 'slow' | 'medium' | 'quick') {
const args = {
pinnedIds,
searchTerm: this.search,
Expand All @@ -435,6 +457,9 @@ export default {
case 'slow':
this.debouncedHelperUpdateSlow(args);
break;
case 'medium':
this.debouncedHelperUpdateMedium(args);
break;
case 'quick':
this.debouncedHelperUpdateQuick(args);
break;
Expand Down
16 changes: 16 additions & 0 deletions shell/store/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,10 @@ export const state = () => {
$router: markRaw({}),
$route: markRaw({}),
$plugin: markRaw({}),
/**
* Cache state of side nav clusters. This avoids flickering when the user changes pages and the side nav component re-renders
*/
sideNavCache: undefined,
};
};

Expand Down Expand Up @@ -613,6 +617,10 @@ export const getters = {
return `${ base }/latest`;
},

sideNavCache(state) {
return state.sideNavCache;
},

...gcGetters
};

Expand Down Expand Up @@ -751,6 +759,10 @@ export const mutations = {

setPlugin(state, pluginDefinition) {
state.$plugin = markRaw(pluginDefinition || {});
},

setSideNavCache(state, sideNavCache) {
state.sideNavCache = sideNavCache;
}
};

Expand Down Expand Up @@ -1270,5 +1282,9 @@ export const actions = {
});
},

setSideNavCache({ commit }, sideNavCache) {
commit('setSideNavCache', sideNavCache);
},

...gcActions
};
2 changes: 1 addition & 1 deletion shell/types/store/vuex.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* Generic interface for Vuex getters
*/
export interface VuexStoreGetters {
[name:string]: Function
[name:string]: Function | any
}

export interface VuexStore {
Expand Down

0 comments on commit 4647776

Please sign in to comment.