- 
                Notifications
    You must be signed in to change notification settings 
- Fork 1.2k
[Navigation] Update styles for v11 #9332
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
Conversation
60428b0    to
    610cfdd      
    Compare
  
    2a54627    to
    92ae058      
    Compare
  
    | const stickyManager = useMemo(() => new StickyManager(), []); | ||
|  | ||
| const featuresConfig = useMemo( | ||
| () => ({...features, polarisSummerEditions2023: false}), | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
polarisSummerEditions2023 was missing from the FeaturesContext and it was causing some tests to fail.
430cd82    to
    5d5de92      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good! Did a tophat on storybook as well (looks great) but didn't cross reference with the Figma
| Can you update the focus states? We've made the change to outline with outline offset. Here's an example of a focus ring override | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks pretty good just some minor things 👍
|  | ||
| #{$se23} & { | ||
| &::before { | ||
| display: none; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can be content: none so the pseudo element doesn't render. Probably doesn't really matter but less dom elements
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can change it. Both should achieve the same result 👍🏻

WHY are these changes introduced?
Part of https://github.com/Shopify/polaris-summer-editions/issues/10
WHAT is this pull request doing?
Update Navigation styles for v11
How to 🎩
🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines
Copy-paste this code in
playground/Playground.tsx:🎩 checklist
README.mdwith documentation changes