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

refactor: adjust sizing of MButton info slot #570

Merged
merged 1 commit into from
Dec 12, 2023
Merged

Conversation

berwyn
Copy link
Contributor

@berwyn berwyn commented Nov 30, 2023

Describe the problem this PR addresses

The information slot in MButton currently uses a width property inside a flex container, which causes the text to be cut off in situations where the main slot runs long.

image

Describe the changes in this PR

This PR changes the slot to be sized with flex properties, specifically setting the flex-basis so that contents aren't elided.

image

Other information

@berwyn berwyn requested a review from a team as a code owner November 30, 2023 21:44
Copy link

Deployed Styleguide and Lab.

Notes
  1. Links may take a few minutes to update after PR is opened or commit is pushed.
  2. Links may become invalidated after PR is merged or closed.
  3. Links for all releases and open PRs can be found on the Maker Deploys page.

@berwyn berwyn changed the title Adjust MButton Info Slot Sizing refactor: adjust sizing of MButton info slot Nov 30, 2023
@ashjtan
Copy link
Collaborator

ashjtan commented Nov 30, 2023

jw, what is the behavior when the main label (left side) is really really long? does the font size accommodate, so that the text continues to wrap? does it get cut off after two lines?

i believe the #info slot (right side) is generally "secondary" to the main label (left side). if we are going to ensure that it always shows via fit-content, we should ensure that the main label always shows as well.

@berwyn
Copy link
Contributor Author

berwyn commented Nov 30, 2023

Both slots are supposed to ellipsize but I'm not sure the logic is correctly applying

@ashjtan
Copy link
Collaborator

ashjtan commented Dec 1, 2023

@berwyn Both slots are supposed to ellipsize but I'm not sure the logic is correctly applying

Do you mind verifying this behavior? I think it's even more important that this works correctly if we're going to force the info slot to always fit-content. It could force a lot of existing MButtons to overflow that weren't before.

@berwyn
Copy link
Contributor Author

berwyn commented Dec 6, 2023

The code shows that both slots should have the class applied, however there aren't any wrapping rules applied so it looks like the text on both slots will wrap, and after all space is exhausted only then will they ellipsise.

We can see that behaviour in the screenshot as well as the site it's hosted from, where the presence of the info slot causes the default slot to wrap with or without the changeset.

@pretzelhammer
Copy link
Collaborator

note: if your pr title begins with "refactor" i believe merging it won't produce a maker release, if you'd like these changes to be released immediately after merging consider changing the pr title to begin with "fix" if it fixes an issue

@berwyn berwyn merged commit ca5bdc9 into master Dec 12, 2023
4 checks passed
@berwyn berwyn deleted the mbutton-info-sizing branch December 12, 2023 23:20
Copy link

🎉 This PR is included in version 19.6.8 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants