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

VerticalDivider for PageLayout.Pane always initially renders as style 'none' #3331

Closed
Bestra opened this issue May 26, 2023 · 4 comments
Closed
Assignees
Labels
bug Something isn't working component: SplitPageLayout Issues related to the PageLayout and SplitPageLayout components react Stale

Comments

@Bestra
Copy link
Contributor

Bestra commented May 26, 2023

Description

I'm using React Router to render several different pages that are laid out almost identically. They generally look like this:

<PageLayout>
  <PageLayout.Pane divider="line">
    {someContent}
  </PageLayout.Pane>
</PageLayout>

Each route renders a component with the same PageLayout.pane with a 'line' divider. When I switch routes, most of the DOM appears stable to my eyes, but there's a brief period where the vertical divider seems to disappear.

I investigated and found that this happens on initial render, and that I could recreate it via the storybook locally. In the video below I've set a breakpoint when the VerticalDivider renders. You'll see that in the first few renders, the divider doesn't show at all. On the third render it pops into view as a line the way I'd expect.

pagelayout-divider-rerenders.mp4

From the React devtools profiler , the VerticalDivider renders a total of 4 times. On the third render it finally becomes visible.

The react devtools profiler. The `VerticalDivider` component's third render of 4 is selected. 'Why did this render?' says 'Hooks 5 and 8 changed'

I set a logpoint in the PageLayout.Pane component to record its props over its renders. You'll see the responsiveVariant value changes from none to line. Each logpoint shows up twice since we're in strict mode.

console.log results. There are 6 lines. variant: {narrow: 'none', narrow: 'line'} is present on every line. responsiveValue is none on the first 4 lines, but line on the last 2. sx: {marginLeft: 0} is present on every line

This corresponds to the useMedia() hooks used by useResponsiveValue() going from false to true.

The hooks tab from the react devtools component window. Hooks 5 and 8 are highlighted as true. The functions for these hooks are the state of theisRegularViewport and isWideViewport from useResponsiveValue

Under the hood useResponsiveValue() calls useMedia() with a defaultState value of false for all three viewport widths

const isNarrowViewport = useMedia(viewportRanges.narrow, false)
const isRegularViewport = useMedia(viewportRanges.regular, false)
const isWideViewport = useMedia(viewportRanges.wide, false)

and this basically forces us to return the fallback value on initial render, which is none for the VerticalDivider.

If I modify useResponsiveValue so that it doesn't pass a default, like this:

  const isNarrowViewport = useMedia(viewportRanges.narrow)
  const isRegularViewport = useMedia(viewportRanges.regular)
  const isWideViewport = useMedia(viewportRanges.wide)

The component initially renders as I'd expect. Would a change like this be possible or will it create problems for SSR? I see this comment:

// Prevent a React hydration mismatch when a default value is provided by not defaulting to window.matchMedia(query).matches.
if (defaultState !== undefined) {
return defaultState
}

Steps to reproduce

  1. Open the storybook and navigate to /?path=/story/components-pagelayout--default
  2. Set your Pane.divider.regular to be line
storybook props for `Pane.divider.regular`, the value is set to `line` 3. Remount the component by clicking the button on the top left The button on the top left of the storybook window. It shows two arrows flowing in a circle.

Version

v35.25.1

Browser

Chrome

@Bestra Bestra added the bug Something isn't working label May 26, 2023
@lesliecdubs lesliecdubs added the component: SplitPageLayout Issues related to the PageLayout and SplitPageLayout components label May 29, 2023
@amrinder1189
Copy link

Hi @lesliecdubs @Bestra , can you please assign this bug to me .

@lesliecdubs
Copy link
Member

@amrinder1189 thanks for offering to contribute! I'll assign to you. Let us know if you have questions while you work on this.

Copy link
Contributor

Hi! This issue has been marked as stale because it has been open with no activity for 180 days. You can comment on the issue or remove the stale label to keep it open. If you do nothing, this issue will be closed in 7 days.

Copy link
Contributor

Hi! This issue has been marked as stale because it has been open with no activity for 180 days. You can comment on the issue or remove the stale label to keep it open. If you do nothing, this issue will be closed in 7 days.

@github-actions github-actions bot added the Stale label May 25, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working component: SplitPageLayout Issues related to the PageLayout and SplitPageLayout components react Stale
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants