OfficeDev / office-ui-fabric-core

The front-end CSS framework for building experiences for Office and Microsoft 365.
https://developer.microsoft.com/en-us/fabric
Other
3.78k stars 465 forks source link

LTR/RTL icon defaults #1121

Open johannes-z opened 6 years ago

johannes-z commented 6 years ago

Right now certain icons require the dir attribute to be set. You can't always provide this attribute however (e.g. only icon names), and if the attribute isn't set these icons won't work at all (see many issues stating this problem).

I'd suggest using dir="ltr" icons as a default if no attribute is provided, as this is the most common layout. Also as a side note, some website provide a lang attribute which could also help for fallbacks.

mikewheaton commented 6 years ago

Marking this as a documentation issue, since the get started page for Fabric Core doesn't include setting the dir attribute. This attribute is required for the directionality mixins (which the Icon component uses) to work correctly.

Our original solution was to default to LTR and then apply overrides for RTL, but this made it difficult for developers to apply their own custom styles as they had to be more specific than the RTL overrides.

johannes-z commented 6 years ago

Aren't the resulting CSS rules something like this?

[dir='ltr'] .ms-Icon-X:before {
  // ...
}
[dir='rtl'] .ms-Icon-X:before {
  // ...
}

Isn't [dir='ltr'] .ms-Icon-X:before more specific than .ms-Icon-X:before anyway? So this wouldn't stop overrides from working, would it?

.ms-Icon-X:before,
[dir='ltr'] .ms-Icon-X:before {
  // ...
}
[dir='rtl'] .ms-Icon-X:before {
  // ...
}
mikewheaton commented 6 years ago

I remember there being some issue with specificity (it may have been an edge case) but you're right, it shouldn't be a problem here. Particularly with the icon classes, where it's not likely that a developer would override the icon code itself. We should investigate this more and see if we can default to the LTR icon to simplify the developer experience, although setting the dir attribute will still be necessary for most other classes/components in Fabric.

To give some context, here's a look at how defaulting to LTR styles can cause issues with customizing components: https://codepen.io/mikewheaton/pen/NzxZwg