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

Viewport: Store viewport, rotation in globals #23448

Closed
wants to merge 9 commits into from

Conversation

shilman
Copy link
Member

@shilman shilman commented Jul 14, 2023

Closes N/A

What I did

Instead of storing the viewport & rotation in addon state, store them in globals.

This has two benefits:

  1. users can now set the viewport via globals URL parameters:
    a. viewport controls device settings
    b. viewportRotated controls device orientation
  2. gets us a step closer to using globals across our toolbar addons

How to test

  1. Fire up a sandbox
  2. Adjust the viewports in Page stories & note URL changes
  3. Navigate between stories
  4. Navigate to the viewport stories in the sandbox and do the same

@shilman shilman force-pushed the shilman/viewport-globals branch from dd1e3ba to afebe5b Compare July 14, 2023 06:08
Copy link
Member

@yannbf yannbf left a comment

Choose a reason for hiding this comment

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

This looks really good! I think there's a bug with the viewportRotated global though.

if you navigate to:
https://62f0fd349c07f78d10b7c017-fetfklaray.chromatic.com/?path=/story/example-button--primary&globals=viewport:mobile1;viewportRotated:!true

it will remove the rotated global and leave the url like so, not loading the viewport in landscape mode:
https://62f0fd349c07f78d10b7c017-fetfklaray.chromatic.com/?path=/story/example-button--primary&globals=viewport:mobile1

@shilman shilman self-assigned this Jul 17, 2023
@thafryer
Copy link

thafryer commented Nov 6, 2023

@shilman Anything that we can do to help get this merged?

@ndelangen ndelangen added feature request and removed maintenance User-facing maintenance tasks labels Nov 29, 2023
@shilman
Copy link
Member Author

shilman commented Dec 26, 2023

closing in favor of #25321

@shilman shilman closed this Dec 26, 2023
@shilman
Copy link
Member Author

shilman commented Dec 26, 2023

@yannbf i suspect that's #25035

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

Successfully merging this pull request may close these issues.

5 participants