-
Notifications
You must be signed in to change notification settings - Fork 143
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: revamped top-nav component #2392
Conversation
🦋 Changeset detectedLatest commit: 7f9fb91 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ PR title follows Conventional Commits specification. |
@@ -114,7 +114,7 @@ type MenuOverlayProps = { | |||
/** | |||
* JSX Slot for MenuItem or anything else | |||
*/ | |||
children: React.ReactElement[] | React.ReactElement; | |||
children: React.ReactElement[] | React.ReactElement | React.ReactNode; |
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.
TypeScript was yelling with this code:
<MenuOverlay>
{items.map(() => <MenuItem />)}
</MenuOverlay>
const SIDE_NAV_EXPANDED_L1_WIDTH_XL = size['208']; | ||
const SIDE_NAV_EXPANDED_L1_WIDTH_BASE = size['208']; |
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.
Not blocker but todo: check this with design once as it will leave too little space for L3 items
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.
Yeah I've raised a thread.
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.
can you fix it already? Rk had designed sidenav probably ask him and fix?
// add padding to the offsetX to account the "More" menu's width changing due to the selection state (eg: More:ProdctName) | ||
// Currently, hardcoding this to 150, we can make this dynamic too but that's causing layout thrashing | ||
// because first we need to calculate the width of the "More" menu and then update the items | ||
const padding = 150; |
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.
are you sure 150 is enough? because in Full Example on storybook, the magic checkout text will also take more than 150px no?
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.
Yeah checked based "magic checkout" only - we can increase if needed though anyways as the screen size reduces if there are let's say only space for 2 items (max 200px) to show and magic checkout itself may take 150px there's physically not enough space for us to fit any items either way.
const SIDE_NAV_EXPANDED_L1_WIDTH_XL = size['208']; | ||
const SIDE_NAV_EXPANDED_L1_WIDTH_BASE = size['208']; |
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.
can you fix it already? Rk had designed sidenav probably ask him and fix?
* feat: added tabnav and topnav docs * chore: fix build * chore: kamlesh feedback * chore: update menu header/footer margin * chore: remove chevron change * chore: update icon * chore: update menu padding * test: add new topnav tests (#2394) * chore: add new topnav tests * chore: update more to button * chore: update snaps * chore: snaps * chore: revert sidenab width * chore: update snaps * chore: test for null * chore: update
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 7f9fb91:
|
Bundle Size ReportUpdated Components
|
Description
Changes
Additional Information
Component Checklist