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

feat(resizable-sidebar): replace custom sidebar with resizable-panels #7274

Merged
merged 6 commits into from
Apr 17, 2024

Conversation

gatzjames
Copy link
Contributor

@gatzjames gatzjames commented Apr 17, 2024

Highlights:

  • Replaces the custom sidebar with react-resizable-panels
  • Panels can now be rendered conditionally
  • Panels can be resized using the keyboard and are more accessible
  • Panel dimensions are now saved in localStorage instead of db
  • Fixes toggling the sidebar on/off using the keyboard shortcuts and from the app menu View > Toggle Sidebar
  • One tradeoff is that the sidebar now uses percentages instead of pixel units.
    • This simplifies the logic of vertical/horizontal layout since it stays the same e.g. 20/80 will remain 20/80 when the direction changes
    • Could be a problem if we relied on the pixel sizes for whatever reason but I've tested and it works fine for all variations

@gatzjames gatzjames requested a review from a team April 17, 2024 11:25
@gatzjames gatzjames self-assigned this Apr 17, 2024
@jackkav
Copy link
Contributor

jackkav commented Apr 17, 2024

Very cool, couple questions.

  • is this 1:1 with the previous implementation, or are them some behaviour differences besides persistence?
  • Is it worth migrating the old width and height values to localstorage? i guess probably not.

@gatzjames
Copy link
Contributor Author

Very cool, couple questions.

  • is this 1:1 with the previous implementation, or are them some behaviour differences besides persistence?
  • Is it worth migrating the old width and height values to localstorage? i guess probably not.

Good points! Added some more info on the PR.

  • Not 1:1 with the previous implementation since this one uses percentages instead of pixel units
  • I don't think it's worth migrating the old values. Perhaps we should remove them from the workspaceMeta model though. I'll try it out and update the PR

jackkav
jackkav previously approved these changes Apr 17, 2024
@gatzjames gatzjames force-pushed the feat/resizable-panels branch from fd91b32 to b3f5e3e Compare April 17, 2024 12:43
@gatzjames gatzjames merged commit 2ed4069 into Kong:develop Apr 17, 2024
7 checks passed
@gatzjames gatzjames deleted the feat/resizable-panels branch April 17, 2024 13:51
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