Open malykhinvi opened 5 years ago
Should that be fixed, considering addonPanelInRight
is deprecated? If it should, what is the proper fix?
Should we override layout, derived from options with the current one (now it works in opposite direction, if I understand correctly)?
In fact, that issue has nothing to do with deprecated options, but there is a general problem related to option overwrite on story change. Probably related to https://github.com/storybooks/storybook/issues/5857
@shilman after small investigation, I updated the title and description of the bug. There are several ways to fix it, but what is the expected behaviour in the following cases:
1. Only global options are defined
panelPosition: 'right'
'bottom'
panelPosition: 'bottom'
, correct?2. There are custom options, specified for the set of stories
panelPosition: 'right'
; Stories A and B use global options; Story C overrides options - panelPosition: 'bottom'
panelPosition: 'bottom'
, correct?3. There are custom options, specified for the set of stories
panelPosition: 'right'
; Stories A and B use global options; Story C overrides options - panelPosition: 'bottom'
panelPosition: 'right'
, correct?4. There are custom options, specified for the set of stories
panelPosition: 'right'
; Stories A and B use global options; Story C overrides options - panelPosition: 'bottom'
panelPosition: 'bottom'
, correct?Proposal
If I were to fix this issue, and the outcomes of the usecases above are correct, then I would change logic in the layout.js
, so that:
client-api
?)store
)setOptions
only when desired options are not the same as currently used (in callback to STORY_CHANGED
)Let me know, if that makes sense, or if I'm thinking in the completely wrong direction 😄
@malykhinvi I agree this is complex and thanks so much for taking it on!! 🙏
Here's my opinion:
Applying these heuristics to your cases above:
panelPosition: 'bottom'
cc @tmeasday I think you have other ideas about state management
I think this issue could be discussed as part of #5857, but thanks for unpacking it with us @malykhinvi.
To start, I disagree that global and local parameters should be treated differently. Certainly we don't have a way to do so right now and I think it's unnecessarily complex too. I think doing it either way will surprise some users so I would prefer to keep it simple.
(example question: what about chapter level parameters?)
The question in my mind is how the "memory" of a layout option should work. Should it be stateful at all? (yes, probably) and should it override the parameters? (yes, probably too).
So I would probably divide options into two categories (as discussed on #5857) -- those that are modifiable in the UI and thus stateful, and those that are not (such as the title of the SB).
The simple thing to do in my mind is just ensure that if a user has a changed a stateful UI option, the story now has no way to override it. So I would disagree about the outcome of case 3.
IS this still active? checking
Describe the bug Panel position is being reset to what is specified in the global options.
To Reproduce Steps to reproduce the behavior:
panelPosition: 'right'
is set in the options)/?path=/story/button--with-text
bottom
mode/?path=/story/button--with-some-emoji
right
modeExpected behavior Addons panel should be still displayed in the bottom.
Code snippets Looks like the problem is here:
setOptions
is being called after every story change, andpanelPosition
is being reset to what is specified in the options, as...pick(options, Object.keys(layout)),
overrides whatever is inlayout
.System: