Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PanelStack2 'Render active panel only' not working #4918

Closed
rygo11 opened this issue Sep 18, 2021 · 3 comments · Fixed by #4924
Closed

PanelStack2 'Render active panel only' not working #4918

rygo11 opened this issue Sep 18, 2021 · 3 comments · Fixed by #4924

Comments

@rygo11
Copy link

rygo11 commented Sep 18, 2021

Environment

  • Package version(s): 3.49.0
  • Operating System: OSX
  • Browser name and version: Brave, Firefox, Safari

Code Sandbox

The issue happens on demo:
https://blueprintjs.com/docs/#core/components/panel-stack2

Steps to reproduce

  1. Go to https://blueprintjs.com/docs/#core/components/panel-stack2
  2. In demo turn off 'Render active panel only'
  3. Set counter to > 0
  4. open next panel
  5. return to panel 1

Actual behavior

Counter value resets to 0 on return to panel 1

Expected behavior

Counter value should remain unchanged

Note I could be wrong about how this component is supposed to work, but my understanding was that by keeping all panels mounted the state of each panel should be maintained.

Possible solution

@adidahiya
Copy link
Contributor

At the time of writing, the core version in the docs site was 3.49.0 (note this version is important to include in your bug report, instead of simply "whatever is in the online demo", since the latter changes over time).

This seems to be a regression in PanelStack2; the counter state should be retained as you navigate through panels when renderActivePanelOnly={false}.

@adidahiya adidahiya changed the title panelStack2 'Render active panel only' not working PanelStack2 'Render active panel only' not working Sep 21, 2021
@adidahiya
Copy link
Contributor

I've traced the problem to this code in PanelView2 which isolates the panel component's lifecycle from that of our PanelStack2 component:

const PanelWrapper: React.FunctionComponent = React.useMemo(
() => () =>
props.panel.renderPanel({
closePanel: handleClose,
openPanel: props.onOpen,
...props.panel.props,
} as PanelProps<T>),
[props.panel.renderPanel, handleClose, props.onOpen, props.panel.props],
);

This was added to fix bugs in controlled mode where we violated some React rules around inconsistent number of hooks called within PanelStack2. But now it seems like the memoization isn't working very well and we create a new wrapper component too often, which causes the wrapped panel to unmount and re-mount excessively. I suppose we shouldn't be using React.useMemo as a semantic guarantee anyway, since the React docs suggest that it should only be considered a performance optimization rather than a guarantee. So we'll need to find another way to create a "PanelWrapper" which guarantees that it doesn't get re-created if a panel definition is unchanged.

@rygo11
Copy link
Author

rygo11 commented Sep 21, 2021

Apologies ref the version number, I now understand the importance and have updated the bug report. Thanks for the follow up.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants