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

260 recalcresize #261

Closed
wants to merge 5 commits into from
Closed

260 recalcresize #261

wants to merge 5 commits into from

Conversation

Psvensso
Copy link
Contributor

@Psvensso Psvensso commented Jan 11, 2024

This PR is created as a discussion/demo to describe #260

The PR flattens the panel group state

From

{
  "group-1-key": {
    "one,two,three": {layout: [33, 33, 34], expandToSizes: {}},
    "one,two": {layout: [50, 50], expandToSizes: {}},
    "two,three": {layout: [50, 50], expandToSizes: {}},
  }
}

To
storing each layout state in a hash

{
  "group-1-key": {
    layout: [33, 33, 34], 
    expandToSizes: {},
    hash: {
      one: 33,
      two: 33,
      three: 34
    }
  } satisfies PanelGroupState
}

The layouts are then always taken from that hash so a baseline is provided when calculating the new state.

layout = panels.map(({ id }) => state?.hash?.[id] || 0)

When then removing a panel the state might me [33, 33] but that will be recalculated.

This is a naive implementation, i have not checked all other use-cases, for example what happends if you drag-n-drop / reorder components then this might have to be revisited.

Copy link

vercel bot commented Jan 11, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-resizable-panels ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 11, 2024 2:46pm

@Psvensso
Copy link
Contributor Author

Of course the "should store layouts separately per panel combination" is exactly what im trying to remove so the tests would then not be valid.

@bvaughn
Copy link
Owner

bvaughn commented Jan 11, 2024

Thank you for the PR! This functionality is working as designed though, for reasons I explained in #260. Thanks for your understnding.

@bvaughn bvaughn closed this Jan 11, 2024
@bvaughn
Copy link
Owner

bvaughn commented Jan 11, 2024

I'm going to re-open this so that I can give it some thought later, but it's not a super high priority for me and I'm not committing to making any changes at this point.

@bvaughn
Copy link
Owner

bvaughn commented Jan 12, 2024

I will still give the proposal you made some consideration, but I don't think I'll go with the change proposed in this branch. It results in unstable sizes.

Compare the behavior of the latest release...

Kapture.2024-01-12.at.18.58.12.mp4

To the behavior in this branch...

Kapture.2024-01-12.at.18.59.07.mp4

Going to close this for now.

@bvaughn bvaughn closed this Jan 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants