-
Notifications
You must be signed in to change notification settings - Fork 221
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
docs: Refreshing GlobalHeader Storybook Example #2891
docs: Refreshing GlobalHeader Storybook Example #2891
Conversation
…ader-example-polish
Note crashes after clicking inside.
I also move the Combobox into it's own component. I'd like to do the same for a NotificationLiveBadge.
Removed unused imports and parameters.
{filteredTasks.length === 0 ? ( | ||
<StyledMenuItem as="span">No Results Found</StyledMenuItem> | ||
) : ( | ||
filteredTasks.map(i => ( |
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.
Although mapping works, we should probably use a render function like in the auto complete example:
{model.state.items.length > 0 && (
<Combobox.Menu.List cs={styleOverrides.comboboxMenuList}>
{item => <Combobox.Menu.Item>{item}</Combobox.Menu.Item>}
</Combobox.Menu.List>
)}
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'll need some help with using this model!
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.
Lets clean up some of the styles and code examples. Content wise it's great! Thank you as always for working through making our documentation better
const GlobalHeaderItem = createComponent('div')({ | ||
displayName: 'GlobalHeader.Item', | ||
Component: ({gap = 's', ...props}: HeaderItemProps, ref) => ( | ||
<Flex gap={gap} alignItems="center" marginX={space.xs} ref={ref} {...props} /> | ||
<Flex gap={gap} alignItems="center" marginX={system.space.x3} ref={ref} {...props} /> |
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.
does passing a token here work?
Workday/canvas-kit Run #8030
Run Properties:
|
Project |
Workday/canvas-kit
|
Branch Review |
william-global-header-example-polish
|
Run status |
Passed #8030
|
Run duration | 03m 41s |
Commit |
2d67180555 ℹ️: Merge 06bf5c5968ec2b44d1e94dc5e5ea3eef0501ff30 into 101b613622d102220b6c00ae496e...
|
Committer | William Stanton |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
27
|
Pending |
24
|
Skipped |
0
|
Passing |
1093
|
View all changes introduced in this branch ↗︎ |
UI Coverage
21.77%
|
|
---|---|
Untested elements |
1640
|
Tested elements |
454
|
Accessibility
99.17%
|
|
---|---|
Failed rules |
5 critical
5 serious
0 moderate
2 minor
|
Failed elements |
180
|
Summary
Fixes: none
The Storybook example
GlobalHeader
had some opportunities for improved accessibility.Tooltip
to icon buttonsSearchField
withCombobox
AriaLiveRegion
to demonstrate screen reader supportedCountBadge
Release Category
Documentation, Examples
Checklist
ready for review
has been added to PRFor the Reviewer
Where Should the Reviewer Start?
Areas for Feedback? (optional)
Testing Manually
Screenshots or GIFs (if applicable)
Thank You Gif (optional)