fix(material/icon): mat-icon misaligned when text-spacing is applied #29686

essjay05 · 2024-09-03T22:30:22Z

Fixes an issue with Angular Components Material Icon component where when text-spacing is applied the icon gets misaligned and if inside of an input or container with an outline, the icon gets cut off. This fix adds css styling to keep the icon middle-aligned when text-spacing is applied.

BEFORE screenshot
AFTER screenshot
AFTER screencast

Fixes b/250063405

github-actions · 2024-09-03T23:16:44Z

Deployed dev-app for 151a279 to: https://ng-dev-previews-comp--pr-angular-components-29686-dev-jyg9okrc.web.app

Note: As new commits are pushed to this pull request, this link is updated after the preview is rebuilt.

crisbeto · 2024-09-19T07:03:11Z

It seems like this causes the icons to bleed out from the button:

Is there a way that we can undo the text spacing just on icons instead?

essjay05 · 2024-10-01T23:56:52Z

It seems like this causes the icons to bleed out from the button:

Is there a way that we can undo the text spacing just on icons instead?

I've made updates to the CSS to target icon to not be affected by text-spacing and it looks like it does somewhat help, but when really shrinking the window at certain points even the text spills out (as does the icons):

Open to any ideas/suggestions you may have @crisbeto.

crisbeto · 2024-10-02T17:05:18Z

src/material/icon/icon.scss

@@ -45,6 +45,13 @@ mat-icon {
  }
 }

+.mat-icon:has(div[style*='letter-spacing:']):has(div[style*='word-spacing:']) {


I think this will only work for inline styles.

Made an update to be targeted similarly to overflow: visible, but the text-spacing styles applied by Parashu still seems to be overriding the .mat-icon !important styles.

I was wondering if it would be acceptable to hide the icons on mobile and add documentation that the icon will disappear so they will need to make sure the aria-label is appropriate even if the icon is missing? Let me know your thoughts @crisbeto.

crisbeto · 2024-10-04T11:39:14Z

src/material/icon/icon.scss

+.mat-grid-tile .mat-icon,
+.mat-mdc-list-item .mat-icon,
+.mat-mdc-list-option .mat-icon,
+.mat-icon:has(div[style*='letter-spacing:']):has(div[style*='word-spacing:']) {


I think that we should drop the .mat-icon:has(div[style*='letter-spacing:']):has(div[style*='word-spacing:'] style. If there's some internal app setting these as inline styles, we should fix the app rather than our styles.

crisbeto · 2024-10-04T11:41:54Z

src/material/icon/icon.scss

+.mat-icon:has(div[style*='letter-spacing:']):has(div[style*='word-spacing:']) {
+  overflow: visible;
+  // Apply styles to avoid text spacing
+  letter-spacing: 0 !important;


Why do we need to add the !important? We can apply these as regular styles on mat-icon. If the app happens to be overriding them, it's on them to remove the overrides.

Hi @crisbeto, I was attempting to have the specificity+!important override the injected stylesheet and/or user agent stylesheets which Parashu and Text Spacing Editor (Chrome extensions) applied styles since it looks like they both target * with letter-spacing, word-spacing, and line-height with !important flags. See Parashu override screenshot and Text Space Editor override screenshot for examples.

Regardless, it looks like if there are external/user agent stylesheets using !important that those are unable to be overridden according to this article I found.

I've gone ahead and removed the targets to inline-styles as well as the usage of !important based on your recommendations.

Let me know if I should change anything else.

crisbeto · 2024-10-08T07:46:58Z

src/material/icon/icon.scss

+
+// Makes button icon visibility hidden to fix b/250063405 to avoid
+// overrunning into sibling/neighboring elements.
+@media (variables.$xsmall) {


I don't think this is an assumption we can make. Even if the screen is small, there can be enough space to show the icon.

Hi @crisbeto! Removed and ended up making updates to .mdc-button min-width/min-height in latest change 46dcaf2.

Before Screenshot
Updated AFTER Screenshot

Let me know if this is acceptable.

crisbeto · 2024-10-08T07:47:30Z

src/material/icon/icon.scss

+.mat-mdc-form-field .mat-icon,
+.mat-grid-tile .mat-icon,
+.mat-mdc-list-item .mat-icon,
+.mat-mdc-list-option .mat-icon {


I still don't understand why this is targeting only icons inside buttons, form fields etc, instead of all icons.

Removed with latest change.

crisbeto · 2024-10-09T13:01:58Z

src/material/button/button.scss

@@ -23,7 +23,8 @@
  align-items: center;
  justify-content: center;
  box-sizing: border-box;
-  min-width: 64px;


I expect this change will cause lots of internal buttons to collapse in size.

Reverted this change.

crisbeto · 2024-10-09T13:02:29Z

src/material/icon/icon.scss

+  // Undoes previous fix to Issue #11826 in preference of
+  // fixing to improve accessibility by not having the icon
+  // get cut off
+  overflow: visible;


I expect this to be quite breaking. Couldn't we resolve the issue by setting something like this?

.mat-mdc-button-base .mat-icon { flex-shrink: 0; }

Updated based on recommendation.

I expect this to be quite breaking. Couldn't we resolve the issue by setting something like this?

.mat-mdc-button-base .mat-icon { flex-shrink: 0; }

Hi @crisbeto, I know you already approved this, but as I was going back and re-testing the approved changes on the buttons, the icons are still getting cut off on the bottom as you can see from this screencast. The flex-shrink: 0 only fixes the icon from being cropped/cutoff on the left/right, but not the bottom.

Let me know your thoughts.

Fixes an issue with Angular Components Material Icon component where when text-spacing is applied the icon gets misaligned and if inside of an input or container with an outline, the icon gets cut off. This fix adds css styling to keep the icon middle-aligned when text-spacing is applied. Fixes b/250063405

Updates styles for Angular Material Icon so when the icon is within a button it sets the overflow to visible to avoid the icon from being cutoff. Also updates the line-height to 1 set Fixes b/250063405

Updates previous fix to move added styles to bottom of scss file for overriding. Fixes b/250063405

Updates classes to target .mat-icon usage in other Angular Material Components (ie. list-item, list-option, grid-tile, form-field, tab, toolbar, and tooltip). Fixes b/250063405

Updates previous fix to target instances where its parent element has or applied to it and to override any , , or changes to the so it should stay the same. Fixes b/250063405

Updates previous fix to Angular Material Icon component to abide by the lint formatting. Fixes b/250063405

Updates previous change to consolidate the important styles that were added to ignore any externally placed text-spacing styles be added to be targeted by all .mat-icon situations. Fixes b/250063405

Updates previous fix to remove the inline targets and the usage of forced usage of styles as it is on the app to remove any overriding styles. Fixes b/250063405 BREAKING CHANGE: removes use of forced styles Updates the previous fix which uses "important" and removes it as any overrides should be fixed by the app mat-icon is used in instead.

Updates previous fix by updating documentation on the usage of mat-icon within mdc-button to notify developers that mat-icon is set to be hidden on mobile/xsmall screens to improve accessibility and recommends the usage of aria-label or aria-labelledby to notify screenreaders of the information that mat-icon is supposed to convey. Fixes b/250063405

Updates previous changes to icon.scss to remove unnecessary specificity targeting and just moves classes to be generalized under .mat-icon. Adds min-height and min-width to .mdc-button so that when buttons (with icons especially) do not allow its content to overflow outside of the button and maintaining its accessibility by not allowing overlapping of text/content. Fixes b/250063405

Updates notation to style change in icon.scss which undoes a previous bug fix for GH issue angular#11826. As this change is now deemed as an accessibility issue due to it being cut off/hidden. Fixes b/250063405 BREAKING CHANGE: undoes hiding icon overflow Reverts icon having its overflow visible to be more accessible.

…ext-spacing Updates previous fix to revert removal of overflow: hidden to remove the breaking change and updates the method of ensuring .mat-icon remains visible by applying flex-shrink to it so the icon does not get cut off and improves accessibility. Fixes b/250063405

essjay05 requested a review from a team as a code owner September 3, 2024 22:30

essjay05 requested review from amysorto and andrewseguin and removed request for a team September 3, 2024 22:30

essjay05 marked this pull request as draft September 3, 2024 22:37

jelbourn added the dev-app preview When applied, previews of the dev-app are deployed to Firebase label Sep 3, 2024

essjay05 force-pushed the fix-text-spacing-cutoff-icon branch from 8527fc2 to 2030bac Compare September 4, 2024 17:23

essjay05 marked this pull request as ready for review September 4, 2024 18:18

essjay05 force-pushed the fix-text-spacing-cutoff-icon branch 4 times, most recently from e516f0a to c2f58cf Compare September 18, 2024 17:59

essjay05 force-pushed the fix-text-spacing-cutoff-icon branch 2 times, most recently from b1faf18 to 4348041 Compare September 24, 2024 17:53

essjay05 force-pushed the fix-text-spacing-cutoff-icon branch from 4348041 to 7b43d83 Compare October 1, 2024 21:34

essjay05 force-pushed the fix-text-spacing-cutoff-icon branch from 5ea9344 to 275d78e Compare October 2, 2024 17:00

crisbeto reviewed Oct 2, 2024

View reviewed changes

essjay05 force-pushed the fix-text-spacing-cutoff-icon branch from 0570000 to 8bc3834 Compare October 3, 2024 21:32

crisbeto reviewed Oct 4, 2024

View reviewed changes

essjay05 force-pushed the fix-text-spacing-cutoff-icon branch from 8bc3834 to 1901dea Compare October 7, 2024 21:56

angular-robot bot added the detected: breaking change PR contains a commit with a breaking change label Oct 7, 2024

crisbeto reviewed Oct 8, 2024

View reviewed changes

essjay05 force-pushed the fix-text-spacing-cutoff-icon branch from 75815cb to c421855 Compare October 8, 2024 20:56

essjay05 requested a review from crisbeto October 9, 2024 00:32

crisbeto reviewed Oct 9, 2024

View reviewed changes

essjay05 force-pushed the fix-text-spacing-cutoff-icon branch from 1c1680b to f6558d6 Compare October 9, 2024 19:16

essjay05 requested a review from crisbeto October 9, 2024 19:17

crisbeto approved these changes Oct 9, 2024

View reviewed changes

essjay05 force-pushed the fix-text-spacing-cutoff-icon branch 2 times, most recently from e9c8daf to 4c0c338 Compare October 16, 2024 18:43

essjay05 force-pushed the fix-text-spacing-cutoff-icon branch from 4c0c338 to b3815cc Compare October 21, 2024 16:30

essjay05 added 12 commits October 23, 2024 16:48

fix(material/icon): changes overflow to visible for button icon

b0b5baa

Updates styles for Angular Material Icon so when the icon is within a button it sets the overflow to visible to avoid the icon from being cutoff. Also updates the line-height to 1 set Fixes b/250063405

refactor(material/icon): moves styles to bottom of scss file

cad33d7

Updates previous fix to move added styles to bottom of scss file for overriding. Fixes b/250063405

fix(material/icon): updates to include .mat-icon variants

427d120

Updates classes to target .mat-icon usage in other Angular Material Components (ie. list-item, list-option, grid-tile, form-field, tab, toolbar, and tooltip). Fixes b/250063405

fix(material/icon): added styling for icon to ignore text-spacing

e2d14ae

Updates previous fix to target instances where its parent element has or applied to it and to override any , , or changes to the so it should stay the same. Fixes b/250063405

refactor(material/icon): fixed lint errors

c11a246

Updates previous fix to Angular Material Icon component to abide by the lint formatting. Fixes b/250063405

refactor(material/icon): consolidate styles to ignore text-spacing

0f9c678

Updates previous change to consolidate the important styles that were added to ignore any externally placed text-spacing styles be added to be targeted by all .mat-icon situations. Fixes b/250063405

essjay05 force-pushed the fix-text-spacing-cutoff-icon branch from b3815cc to 151a279 Compare October 23, 2024 16:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(material/icon): mat-icon misaligned when text-spacing is applied #29686

fix(material/icon): mat-icon misaligned when text-spacing is applied #29686

essjay05 commented Sep 3, 2024 •

edited

Loading

github-actions bot commented Sep 3, 2024 •

edited

Loading

crisbeto commented Sep 19, 2024

essjay05 commented Oct 1, 2024

crisbeto Oct 2, 2024

essjay05 Oct 3, 2024

crisbeto Oct 4, 2024

crisbeto Oct 4, 2024

essjay05 Oct 7, 2024

crisbeto Oct 8, 2024

essjay05 Oct 8, 2024

crisbeto Oct 8, 2024

essjay05 Oct 8, 2024

crisbeto Oct 9, 2024

essjay05 Oct 9, 2024

crisbeto Oct 9, 2024

essjay05 Oct 9, 2024

essjay05 Oct 23, 2024

fix(material/icon): mat-icon misaligned when text-spacing is applied #29686

Are you sure you want to change the base?

fix(material/icon): mat-icon misaligned when text-spacing is applied #29686

Conversation

essjay05 commented Sep 3, 2024 • edited Loading

github-actions bot commented Sep 3, 2024 • edited Loading

crisbeto commented Sep 19, 2024

essjay05 commented Oct 1, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

essjay05 commented Sep 3, 2024 •

edited

Loading

github-actions bot commented Sep 3, 2024 •

edited

Loading