UI: Fix enableShortcuts, exclude it from being persisted

[ndelangen]

Closes #6569

What I did

fix issue where if setConfig is called before channel is setup it no-ops
fix issue where setConfig would not call with full value
fix issue where state.ui is persisted where it should not

How to test

• set enableShortcuts: true in manager.js, check if keyboardshortcuts work
• set enableShortcuts: false in manager.js, check if keyboardshortcuts are indeed no longer working
• set enableShortcuts: true in manager.js, check if keyboardshortcuts wokring again.

Between each step you will have to restart storybook.

[tmeasday]

Can you explain this change? Seems sort of wide ranging?

[ndelangen]

storybook/code/lib/types/src/modules/api.ts

Lines 105 to 109 in 42fae30

	export interface API_UI {
	name?: string;
	url?: string;
	enableShortcuts: boolean;
	}

According to the types, these are the only options allowed in this object... And I think none of these should really be persisted!

[tmeasday]

The main part of this seems good. Very unsure about the persistence change though.

[shilman]

@ndelangen Thanks for fixing this! The code looks OK for me.

However, as I was testing it, I found that I needed to do a hard reload in the browser after restarting Storybook, which still feels like a bug. Is there any way we can fix this?

I also tested a few of the other UI options and they don't work. Can open a separate issue if you think it's unrelated to this PR:

  isFullscreen: true,
  panelPosition: 'bottom',
  showRoots: false,

[ndelangen]

@tmeasday I found that anything you put into manager.js is basically going to get ignored forever if it's already persisted.
Which makes sense, for stuff you can change in storybook, but it makes no sense for the state.ui part, I think.

However, as I was testing it, I found that I needed to do a hard reload in the browser after restarting Storybook, which still feels like a bug.

I assume when you say "restarting storybook" that includes a browser refresh? So you're saying you had to refresh twice to see the effect? That would be a bug, though I'm not sure what the problem is...

[ndelangen]

@shilman what you found, is related to persistence, and it's an existing issue.

What happens is:

• upon first browser opening, manager.js is loaded, and it immediately sets persistence in localStorage.
• upon subsequent browser openings, manager.js is loaded, but it's values are overridden by the persisted data.
• if manager.js changes, this doesn't change anything about the above.

TLDR:
Once the data is persisted, It cannot be overridden anymore.

This PR, fixes that for enableShortcuts by not persisting the .ui property anymore.
Because enableShortcuts can't be changed in the UI anywhere at all, this seems like a good choice.

[ndelangen]

@shilman I've verified, that setting:

addons.setConfig({
  panelPosition: 'right',
});

...will work, only if you clear site data first.

I'm not sure what else to expect..
We could have a long discussion about exactly what should be persisted when, but it feels like a non-7.0 change?

[shilman]

Let's merge this and figure out a proper solution to this later