feat: #153 v5 top bar update docs #240
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
nurm717123
changed the title
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferencesCodacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more Footnotes
|
|
I have re-updated the top bar docs to include stories for each viewport variant, as suggested by the design team, there is 2 option to go through each viewport variant:
I keep both version, so let me know if we can't keep both |
| @@ -34,7 +34,7 @@ export const ElTopBar = styled.div` | |||
| border-bottom: var(--border-default, 1px) solid var(--outline-default, #e5e9ed); | |||
| background: var(--fill-white, #fff); | |||
|
|
|||
| ${isDesktopOrBelow} { | |||
| ${isBelowWideScreen} { | |||
There was a problem hiding this comment.
I initially overlooked this because there are no < or > indicators in the design. However, I now believe this is correct since the only viewport with an indicator is mobile (<768px). This implies that tablet starting at 768px should mean 768px++, and similarly, desktop should be 1024px++, and so on.
| @@ -46,7 +46,7 @@ export const ElTopBar = styled.div` | |||
| } | |||
| } | |||
|
|
|||
| ${isTabletOrBelow} { | |||
| ${isBelowDesktop} { | |||
There was a problem hiding this comment.
same as above
I initially overlooked this because there are no < or > indicators in the design. However, I now believe this is correct since the only viewport with an indicator is mobile (<768px). This implies that tablet starting at 768px should mean 768px++, and similarly, desktop should be 1024px++, and so on.
| import { INITIAL_VIEWPORTS } from '@storybook/addon-viewport' | ||
| console.log(INITIAL_VIEWPORTS) | ||
|
|
||
| const viewports: typeof INITIAL_VIEWPORTS = { |
There was a problem hiding this comment.
need to add additionals because storybook's provided viewport doesn't have widescreen .etc
| }, | ||
| }, | ||
| } | ||
| export const ReactUsage: StoryObj<typeof TopBar> = { | ||
|
|
||
| export const ResponsiveMainNav: Story = { |
There was a problem hiding this comment.
all stories already responsive since it use React component now, this one is different because containing CSSQueryContainer which doesn't show in the other one
| @@ -11,7 +11,7 @@ export const GuidelinesImage: FC<GuidelinesImageProps> = ({ name }) => ( | |||
|
|
|||
| export const figmaDesignUrls = { | |||
| appBar: | |||
| 'https://www.figma.com/design/6CaivqdlTX0UkFYJkpBKDu/Reapit-DS?node-id=428-7209&node-type=frame&t=RHaBJeesRCccFe53-0', | |||
| 'https://www.figma.com/design/XJ6qcAV8gHscsUodqJMNEF/Reapit-Elements-production-ready-components?node-id=16-4840&m=dev', | |||
There was a problem hiding this comment.
TopBar related component now use this docs as main source
In line with our previous team discussions about standardizing the MDX documentation and avoiding the embedding of Figma guideline SVGs, this pull request addresses that issue. Additionally, this pr also contain small fix regarding the default top-bar menu popover y offset, I think there was a potential misunderstanding regarding the default Y offset of the menu, specifically:
previous reference
which should be this (notice there is wider gap between nav and menu)
thankfully to make that gap size we can utilize
spacing-10to have the same space 👍 .Pull request checklist
Detail as per issue below (required):
fixes:
fixed result:
menu gap size
docs
2024-12-06.13-53-30.mov