From 8dcc72299ba10b338ef22eb2fc6c7cb682bde6b9 Mon Sep 17 00:00:00 2001 From: Richard Cox Date: Mon, 6 Jan 2025 12:32:44 +0000 Subject: [PATCH] Two side nav improvements - Reduce the flicker of cluster icons when the top level menu component is recreated given a change to the page's layout - Add finer changes and comments to reduce churn from user / system changes to resources --- shell/components/nav/TopLevelMenu.helper.ts | 14 +++++++ shell/components/nav/TopLevelMenu.vue | 45 ++++++++++++++++----- shell/store/index.js | 16 ++++++++ shell/types/store/vuex.d.ts | 2 +- 4 files changed, 66 insertions(+), 11 deletions(-) diff --git a/shell/components/nav/TopLevelMenu.helper.ts b/shell/components/nav/TopLevelMenu.helper.ts index 1c886ef77f4..706e1789a43 100644 --- a/shell/components/nav/TopLevelMenu.helper.ts +++ b/shell/components/nav/TopLevelMenu.helper.ts @@ -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 { @@ -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 }); + } } /** @@ -258,6 +268,8 @@ export class TopLevelMenuHelperPagination extends BaseTopLevelMenuHelper impleme this.clustersPinned.push(..._clustersPinned); this.clustersOthers.push(..._clustersNotPinned); + + this.cacheClusters(); } private constructParams({ @@ -401,6 +413,8 @@ export class TopLevelMenuHelperLegacy extends BaseTopLevelMenuHelper implements this.clustersPinned.push(..._clustersPinned); this.clustersOthers.push(..._clustersNotPinned); + + this.cacheClusters(); } /** diff --git a/shell/components/nav/TopLevelMenu.vue b/shell/components/nav/TopLevelMenu.vue index 829ba829d3e..b5905a2d56c 100644 --- a/shell/components/nav/TopLevelMenu.vue +++ b/shell/components/nav/TopLevelMenu.vue @@ -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, @@ -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, }; @@ -264,6 +276,14 @@ 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) { @@ -271,27 +291,29 @@ export default { 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, @@ -424,7 +446,7 @@ export default { }; }, - updateClusters(pinnedIds, speed = 'slow' | 'quick') { + updateClusters(pinnedIds, speed = 'slow' | 'medium' | 'quick') { const args = { pinnedIds, searchTerm: this.search, @@ -435,6 +457,9 @@ export default { case 'slow': this.debouncedHelperUpdateSlow(args); break; + case 'medium': + this.debouncedHelperUpdateMedium(args); + break; case 'quick': this.debouncedHelperUpdateQuick(args); break; diff --git a/shell/store/index.js b/shell/store/index.js index ed570362eb0..89367962649 100644 --- a/shell/store/index.js +++ b/shell/store/index.js @@ -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, }; }; @@ -613,6 +617,10 @@ export const getters = { return `${ base }/latest`; }, + sideNavCache(state) { + return state.sideNavCache; + }, + ...gcGetters }; @@ -751,6 +759,10 @@ export const mutations = { setPlugin(state, pluginDefinition) { state.$plugin = markRaw(pluginDefinition || {}); + }, + + setSideNavCache(state, sideNavCache) { + state.sideNavCache = sideNavCache; } }; @@ -1270,5 +1282,9 @@ export const actions = { }); }, + setSideNavCache({ commit }, sideNavCache) { + commit('setSideNavCache', sideNavCache); + }, + ...gcActions }; diff --git a/shell/types/store/vuex.d.ts b/shell/types/store/vuex.d.ts index 4d0fe8a47d8..7ae015439bb 100644 --- a/shell/types/store/vuex.d.ts +++ b/shell/types/store/vuex.d.ts @@ -5,7 +5,7 @@ * Generic interface for Vuex getters */ export interface VuexStoreGetters { - [name:string]: Function + [name:string]: Function | any } export interface VuexStore {