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

🔧 supporting different width behaviors for breakpoints #1287

Merged
merged 14 commits into from
Feb 22, 2023

Conversation

tesk9
Copy link
Contributor

@tesk9 tesk9 commented Feb 21, 2023

Fixes A11-2309

Context

The goal is to be able to specify how the Menu trigger width should behave at different breakpoints, so that we don't have nice-looking desktop views and janky-looking mobile views. See #1265 for some example screenshots.

Since Menu now integrates directly with Button and the other clickables, I'm addressing this need by adding per-viewport width support to Button.

I don't think we'll want clickable texts to expand to take up tons of horizontal space, so I'm not making changes the ClickableText component.

I also think the width behavior of clickable svgs is a bit less complicated? And it makes sense to let the user use mobileCss directly. Example of doing this is here: https://github.com/NoRedInk/NoRedInk/pull/42777.

Additionally, I wanted the width behavior to be easy to test and observe, so I split out an interactive example from the Button table, and made the Button table entirely non-interactive.

🖼️ What does this change look like?

Width behavior by breakpoint demo

width.settings.demo.mov

Examples Table Before

image

Examples Table After

image

cc @NoRedInk/design

Component completion checklist

  • I've gone through the relevant sections of the Development Accessibility guide with the changes I made to this component in mind
  • Changes are clearly documented
    • Component docs include a changelog
    • Any new exposed functions or properties have docs
  • Changes extend to the Component Catalog
    • The Component Catalog is updated to use the newest version, if appropriate
    • The Component Catalog example version number is updated, if appropriate
    • Any new customizations are available from the Component Catalog
    • The component example still has:
      • an accurate preview
      • valid sample code
      • correct keyboard behavior
      • correct and comprehensive guidance around how to use the component
  • Changes to the component are tested/the new version of the component is tested
  • Component API follows standard patterns in noredink-ui
    • e.g., as a dev, I can conveniently add an nriDescription
    • and adding a new feature to the component will not require major API changes to the comopnent

@tesk9 tesk9 changed the title Bat/menu width breakpoints 🔧 supporting different width behaviors for breakpoints Feb 21, 2023
@tesk9 tesk9 marked this pull request as ready for review February 21, 2023 19:39
@linear
Copy link

linear bot commented Feb 21, 2023

A11-2309 Adjust Menu width behavior by breakpoint

See noredink-ui issue

3 breakpoints (we think)

@bendansby
Copy link
Contributor

bendansby commented Feb 21, 2023

What if we want to use defaultTrigger for Menus, which we generally will?

@tesk9
Copy link
Contributor Author

tesk9 commented Feb 21, 2023

defaultTrigger uses Button under the hood, so you can pass a list of Button attributes when you make it!

@bendansby
Copy link
Contributor

Awesome, thanks for doing this!

@tesk9 tesk9 requested a review from charbelrami February 21, 2023 21:50
Copy link
Contributor

@charbelrami charbelrami left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

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.

3 participants