Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

fix(fab): Change fab to inline-block. Move material-icons class to icon #2692

Closed
wants to merge 1 commit into from

Conversation

williamernest
Copy link
Contributor

Fixes icon alignment issues with IE11. The icon was rendered on the right side of the fab.

Also, the demos were putting the material-icons class on the button instead of on the icon class, which causes IE11 to align the icons higher than it should be.

@codecov-io
Copy link

Codecov Report

Merging #2692 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2692   +/-   ##
=======================================
  Coverage   98.48%   98.48%           
=======================================
  Files          98       98           
  Lines        4230     4230           
  Branches      537      537           
=======================================
  Hits         4166     4166           
  Misses         64       64

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d16a42e...531c947. Read the comment docs.

<button class="mdc-fab material-icons" aria-label="Favorite">
<svg xmlns="http://www.w3.org/2000/svg" class="mdc-fab__icon" viewBox="0 0 24 24" fill="#000000">
<button class="mdc-fab" aria-label="Favorite">
<svg xmlns="http://www.w3.org/2000/svg" class="material-icons mdc-fab__icon" viewBox="0 0 24 24" fill="#000000">
Copy link
Contributor

Choose a reason for hiding this comment

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

The SVG variants shouldn't even need the material-icons class, right?

@@ -58,7 +58,7 @@ $mdc-fab-icon-enter-duration_: 180ms;
@include mdc-ripple-radius-bounded;
@include mdc-elevation(6);

display: inline-flex;
display: inline-block;
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like a heavy-handed change just to fix IE 11. Does Button not have a similar issue? That uses inline-flex as well.

If we change this, justify-content no longer has any meaning and I suspect we should add text-align: center, but this further confuses me as to how this is fixing something without breaking anything else.

@kfranqueiro
Copy link
Contributor

This change causes the icon to be vertically misaligned in Chrome (and vertical alignment is most likely the reason we were using inline-flex all along):

fab html

@williamernest williamernest deleted the fix/fab/fix-ie11-bug branch May 2, 2018 21:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants