Missing overflow-visible on some icon button examples

[hannahiss]

Note: Please transform - [ ] into - (NA) in the description when things are not applicable

Related issues

Description

Add missing class .overflow-visible utility, like the other examples arround.

Motivation & Context

This class is needed to prevent SVG content from being cropped

Types of change

• Bug fix (non-breaking which fixes an issue)

Live previews

• https://deploy-preview-2701--boosted.netlify.app/docs/components/buttons/#with-icon

[netlify]

✅ Deploy Preview for boosted ready!

Name	Link
🔨 Latest commit	6707b2d
🔍 Latest deploy log	https://app.netlify.com/sites/boosted/deploys/670cda17fa0da00008256a46
😎 Deploy Preview	https://deploy-preview-2701--boosted.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[louismaximepiton]

Since our <svg>s have their own security margin, I'm wondering if we still need the explanation.

If you think that we should keep them:

• 20 are missing in dark-mode.md line 1403 up to 1507 for the close buttons.
• 30 are missing in dark-mode.md line 8356 up to 8821 for the close buttons.
• 4 are missing in close-button.md line 43 up to 58 for the close buttons.
• 2 are missing in tooltips.md line 141 up to 157 for the directions.
• ;I think that the svgs inside the theme toggler should be changed and in general .nav-link/.nav-icon. If this is the case, many more are coming and I don't know the limit but maybe almost all svg should get .overflow-visible.
• 2 are missing in download-app line 367 up to 378 for the outline buttons.
• 1 is missing in icons.md line 182 for the outline buttons.
• 6 are missing in checks-radios.md line 315 up to 357 for the no outline buttons.
• 1 is missing in docs.html line 39 for the On this page button.
• 1 is missing in single.html line 19 for the Download button.
• 1 is missing in docs-subnav.html line 6 for the Toggle button.
• 1 is missing in masthead.html line 23 for the Read the docs button.
• 2 are missing in ;card.html line 41 up to 49 for the Sahre and Favorite buttons.
• 2 are missing in example.html line 35 and 38 for the Copy and StackBlitz buttons.
• 1 is missing in js-docs.html line 43 for the Copy button.
• 1 is missing in scss-docs.html line 56 for the Copy button.

On the other side if you think that there are not needed anymore:

• 40 to remove in dark-mode.md.
• 6 to remove in buttons.md.
• 3 to remove in orange-navbar.md.
• 3 to remove in orange-global-headers.html.
• 2 to remove in form.html example.
• 2 to remove in orange-supra.html.

[hannahiss]

Since our <svg>s have their own security margin, I'm wondering if we still need the explanation.

@julien-deramond
I did some testing: removing the .overflow-visible class completely, I couldn't reproduce any issues with SVGs being cropped, so I wonder if we could remove this from the documentation completely? What do you think?

[julien-deramond]

@julien-deramond
I did some testing: removing the .overflow-visible class completely, I couldn't reproduce any issues with SVGs being cropped, so I wonder if we could remove this from the documentation completely? What do you think?

Let’s approach this with a logical deduction based on our current knowledge.

None of us have identified a specific reason why the .overflow-visible class is applied to Boosted's buttons with icons, nor have the history of this specificity. Similarly, we are not aware of any browser-related issues that would cause icons to be cropped in this context.

If we consider how other implementations handle this:

• In Bootstrap's examples, there’s no .overflow-visible rule applied to the "Download examples" button containing an icon.
• Similarly, Adobe’s Spectrum Buttons and other component libraries do not apply this rule either.
• Our own Close button and color theme changer in the header don't have this .overflow-visible class

The only valid use case I can think of is if an icon or SVG has a very specific rendering requirement, for instance:

<button type="button" class="btn btn-primary">
  <svg width="1rem" height="1rem" fill="currentColor" aria-hidden="true" class="me-1">
    <text x="0" y="1.2rem" font-size="1rem">Text</text>
  </svg>
  Primary
</button>

However, our components are intended to be used with Solaris icons or, in the future, Orange’s pre-packaged icon libraries, where all icons are designed to fit uniformly within a square container of consistent size.

Given this, I’d estimate that in 99% of cases, overflow-visible is unnecessary.

This leads to two conclusions:

1. By default, we can safely remove the .overflow-visible class from all buttons containing icons.
2. In the documentation, we can maybe add a note stating that if an SVG is cropped in rare cases, the .overflow-visible class can be applied to fix the rendering. And I'm not 100% certain that it's useful. It's more like to not forget that it existed in the past 😄

What do you think folks?

[hannahiss]

This leads to two conclusions:

1. By default, we can safely remove the .overflow-visible class from all buttons containing icons.
2. In the documentation, we can maybe add a note stating that if an SVG is cropped in rare cases, the .overflow-visible class can be applied to fix the rendering. And I'm not 100% certain that it's useful. It's more like to not forget that it existed in the past 😄

What do you think folks?

I'm in 😄

[julien-deramond]

@hannahiss We should probably add a note in the migration guide as the markup of some components are modified (even if it doesn't have any impacts for the users).

[louismaximepiton]

I'm fine removing this class since it doesn't bring much.

[julien-deramond]

LGTM! Just added a migration note via e063766, please tell me if that's OK for you so that I can merge this PR.