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

UI: Overhaul layout engine #20412

Merged
merged 77 commits into from
Sep 19, 2023
Merged

UI: Overhaul layout engine #20412

merged 77 commits into from
Sep 19, 2023

Conversation

ndelangen
Copy link
Member

@ndelangen ndelangen commented Dec 25, 2022

Resolves #14916
Closes #24215
Closes #24252
Closes #24253

Works on #24124

Storybook.layout.demo.mp4

What we did

This PR completely overhauls the layout engine, and how sidebar/panels are sized and positioned. Mobile/responsive UI does not work here, that's being worked on in follow-up PRs: #24124

  • Switch from using absolute positioning of all panels to using CSS grid.
  • Don't re-render Canvas when resizing to a mobile viewport
  • ... In fact don't use resize observers at all, but rely on matching media queries instead, which is more performant and less error prone.
  • Enable users to drag panels to the edge to toggle them off completely, which is easier than using the menu or remembering keyboard shortcuts - both of which still works. The sidebar and right panel still have a minimum width though.
  • Change the layout state to not have booleans for the panels but instead actual sizes, to ensure you can't get into "impossible states" like { isFullscreen: true, showNav: true }. The new size state properties are accompanies by recentVisisbleSizes properties that are used when restoring panels back from a non-shown state
  • Changed how layout state is synced, so the Layout component has an internal state of the size that is synced to the manager API when dragging stops. There's also a sync the other way, when sizes are changed via the manager API.

This PR was first done by @ndelangen but then finished 6 months later by @JReinhold and @cdedreuille, so not all changes might make sense together anymore.

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

  1. Compile and run any sandbox
  2. Resize panels with dragging
  3. Toggle panels with the menu and with keyboard shortcuts (A, S, D and F)
  4. Use query params to set initial layout state as per documentation.

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli/src/sandbox-templates.ts

  • Make sure this PR contains one of the labels below:

    Available labels
    • bug: Internal changes that fixes incorrect behavior.
    • maintenance: User-facing maintenance tasks.
    • dependencies: Upgrading (sometimes downgrading) dependencies.
    • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
    • cleanup: Minor cleanup style change. Will not show up in release changelog.
    • documentation: Documentation only changes. Will not show up in release changelog.
    • feature request: Introducing a new feature.
    • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
    • other: Changes that don't fit in the above categories.

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>

@ndelangen
Copy link
Member Author

A bit of a Christmas present 🎁

@ghengeveld
Copy link
Member

A bit of a Christmas present 🎁

What are you doing working on Christmas?!

Copy link
Member

@IanVS IanVS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not too familiar with the manager UI, so I don't feel qualified to give this a ✅, but it's definitely pretty exciting looking! I left a few niggly comments and questions, from a brief skim-through.

code/ui/manager/src/components/layout/Layout.tsx Outdated Show resolved Hide resolved
code/ui/manager/src/container/Panel.tsx Outdated Show resolved Hide resolved
code/ui/manager/src/settings/index.tsx Outdated Show resolved Hide resolved
code/ui/manager/src/components/layout/Layout.types.ts Outdated Show resolved Hide resolved
@shilman shilman changed the title Overhaul panel payout Overhaul panel layout Dec 26, 2022
@shilman
Copy link
Member

shilman commented Dec 26, 2022

Amazing work here @ndelangen -- I'm excited to work through the comments and see if we can get this merged for 7.0

Nice suggestions across the board @IanVS! Merry christmas!!! 🎄

getIsNavShown() {
return getIsNavShown(store.getState());
},

setOptions: (options: any) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the changes below just reverses the condition for readability, nothing else.

@@ -284,7 +389,6 @@ export const init: ModuleFn = ({ store, provider, singleStory, fullAPI }) => {
api,
state: merge(api.getInitialOptions(), persisted),
init: () => {
api.setOptions(merge(api.getInitialOptions(), persisted));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this makes setting layout from URL work again, similar to #23750

Copy link
Contributor

@JReinhold JReinhold Sep 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should re-add this back in a later iteration

@JReinhold JReinhold changed the title UI: Overhaul panel layout UI: Overhaul layout engine Sep 19, 2023
@ndelangen
Copy link
Member Author

@JReinhold I assume you're taking this PR to it's conclusion? Do you need anything from me?

@JReinhold JReinhold merged commit a01591e into new-layout Sep 19, 2023
49 of 50 checks passed
@JReinhold JReinhold deleted the norbert/panel-size branch September 19, 2023 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Addons panel does not appear
9 participants