VizPanel: Do not refresh color mode on changePluginType if plugin is the same

[mdvictor]

In most cases we want to refresh the fieldConfig/options when changing plugin type, but in the case of Library Panels, for example, we only want to rerender the panel, without refreshing it's configs, since the existing ones are valid. Through this flag we can fix that here and avoid bugs where a lib panel has a set color mode, but after changePluginType the defaults are applied and the lib panel ones are overwritten

📦 Published PR as canary version: 5.22.0--canary.950.11661930270.0

✨ Test out this PR locally via:

npm install @grafana/scenes-react@5.22.0--canary.950.11661930270.0
npm install @grafana/scenes@5.22.0--canary.950.11661930270.0
# or 
yarn add @grafana/scenes-react@5.22.0--canary.950.11661930270.0
yarn add @grafana/scenes@5.22.0--canary.950.11661930270.0

[torkelo]

Why do we not want refresh config? When the library panel is loaded we get both the type the config , a bit confuse why only plugin would change

[mdvictor]

We don't want to refresh them in that case because we're not changing the panel plugin type, so we don't need any new defaults, we should use the ones from the lib panel.

The problem is that after changePluginType calls _loadPlugin with isAfterPluginChange true, which later on calls adaptFieldColorMode here. If we have color mode threshold set, then on line 178 we shift it back to classic.

I tested some more and it seems no other options/configs are affected other than color palette when set to thresholds.. so maybe a better fix would be in adaptFieldColorMode itself, but not sure yet what

[torkelo]

@mdvictor

so we don't need any new defaults,

Yes we do, why would we not need it? (Say the panel plugin has added some new options that where not saved with the library panel)

[mdvictor]

hmm, that's true, I didn't think of the scenario where new options would be added. Thought we don't need any options overrides because we are changing to the same plugin, the options are the same, unless of course, there are new ones added in the meantime that were not saved in the libPanel, as you pointed out. I'll close this then and look for a different approach.

[mdvictor]

@torkelo I looked more into this, and realized that this flag doesn't actually affect the options (e.g.: here). the isAfterPluginChange flag is only used in adaptFieldColorMode so I actually think now that we could move forward with this since we do not need to adapt the color mode, but keep using what's set in the lib panel

[torkelo]

don't like this function parameter

• Naming it confusing / in-accurate (only affects the isAfterPluginChange param)
• Don't think we need it.

We could instead do

Suggested change

	await this._loadPlugin(pluginId, newOptions ?? {}, newFieldConfig, refreshConfigs);
	// If state.pluginId is already the correct plugin we don't treat this as plain user panel type change
	const isAfterPluginChange = this.state.pluginId !== pluginId;
	await this._loadPlugin(pluginId, newOptions ?? {}, newFieldConfig, isAfterPluginChage);

[mdvictor]

True, this is much better, also no need for a second PR in grafana for calling with that extra param 💯

[scenes-repo-bot-access-token]

🚀 PR was released in v5.22.0 🚀