Skip to content

Conversation

kvendrik
Copy link
Member

@kvendrik kvendrik commented Jan 10, 2019

WHY are these changes introduced?

Updates the frame layout in light of some soon-to-be-introduced features that will live in the frame. Further details can be found here.

Relies on: #849
Design: https://shopify.invisionapp.com/share/5AP64DZRKZX#/screens/331903554

WHAT is this pull request doing?

  • Updates the positioning of the search field.
  • Keeps TopBar.UserMenu visible on mobile.
  • Shares anything passed into Navigation.UserMenu with TopBar.UserMenu. Essentially this means that Navigation.UserMenu visually doesn't exist anymore but can still be used as any props that you pass into it will be used as props for TopBar.UserMenu on mobile.
  • Removes the border-right on the side navigation.
  • Other minor visual changes.

How to 🎩

  1. Once on the branch revert this commit to get the Playground running (the meta tag it adds to Storybook is explained here).
  2. Run yarn dev, head over to the playground and make sure that:
    1. The search field stays left aligned with the page title.
    2. The new top bar layout holds up when you resize your window.
    3. On desktop the user menu in the top bar is the user menu that was fed into TopBar.
    4. On mobile the user menu that was fed into Navigation is shown in the top bar.
    5. Everything works as you would expect.
  3. Repeat step 2 on multiple browsers and devices.

🎩 checklist

Note: ship #887 when this is merged.

@kvendrik kvendrik changed the title [WIP] Updates frame layout Updates frame layout Jan 17, 2019
Copy link
Contributor

@tmlayton tmlayton left a comment

Choose a reason for hiding this comment

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

Code and 🎩 looks good – please squash or rebase into a smaller number of commits before merging though.

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.

3 participants