Skip to content
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

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

williamjstanton
Copy link
Collaborator

Summary

Fixes: none

The Storybook example GlobalHeader had some opportunities for improved accessibility.

  1. Added accessible Tooltip to icon buttons
  2. Replaced old SearchField with Combobox
  3. Used AriaLiveRegion to demonstrate screen reader supported CountBadge
  4. Updated style packages

Release Category

Documentation, Examples

Release Note

Optional release note message. Changelog and release summaries will contain a pull request title. This section will add additional notes under that title. This section is not a summary, but something extra to point out in release notes. An example might be calling out breaking changes in a labs component or minor visual changes that need visual regression updates. Remove this section if no additional release notes are required.

Checklist

For the Reviewer

  • PR title is short and descriptive
  • PR summary describes the change (Fixes/Resolves linked correctly)
  • PR Release Notes describes additional information useful to call out in a release message or removed if not applicable
  • Breaking Changes provides useful information to upgrade to this code or removed if not applicable

Where Should the Reviewer Start?

Areas for Feedback? (optional)

  • Code
  • Documentation
  • Testing
  • Codemods

Testing Manually

Screenshots or GIFs (if applicable)

Thank You Gif (optional)

{filteredTasks.length === 0 ? (
<StyledMenuItem as="span">No Results Found</StyledMenuItem>
) : (
filteredTasks.map(i => (
Copy link
Contributor

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>
              )}

Copy link
Collaborator Author

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!

Copy link
Contributor

@mannycarrera4 mannycarrera4 left a 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

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.

2 participants