Convert all UI code to CSS logical properties #8051
Conversation
thibaudcolas
added
Accessibility
component:i18n
thibaudcolas
changed the title
|
|
||
| &:before { | ||
| font-size: 1rem; | ||
| position: absolute; | ||
| // Remove once we drop support for Safari 13. | ||
| // stylelint-disable-next-line property-disallowed-list |
There was a problem hiding this comment.
Just an idea - should we create a mixin? @include safari { left: 0; }
Something like that might help to be explicit and also allows us to use a media / supports selector to house the enclosed styles.
There was a problem hiding this comment.
We’ve discussed this together at the last UI team meeting – I chose to keep this like this because I’d like it to go away in the next + 1 release (August 2022, so we’ll be able to remove this code after the May 2022 release’s code freeze – late April). Hard-coding the styles like this is the pattern that will be the simplest to remove mechanically in 1 month’s time.
| // 'float': ['inline-start', 'inline-end', 'none', 'unset'], | ||
| 'text-align': ['start', 'end', 'center'], | ||
| }, | ||
| // Disable declaration-strict-value until we are in a position to enforce it. |
There was a problem hiding this comment.
Is the plan to use a newer version of the Wagtail stylelint LIbrary? - let me know if you need me to do a new release of that library to help.
Ignore this if you were just adding this temporarily.
There was a problem hiding this comment.
I chose to fully disable this because the needed refactorings would be distracting to what this PR is trying to achieve (and the PR is already big as-is). Once this gets merged, we can create a follow-up task to gradually enforce this.
|
Thanks for this @thibaudcolas! I'm wondering what the advantage of using CSS logical properties is for Wagtail over width/height/left/right/etc? The existing physical properties are quite easy for most developers to understand. I'm concerned that unless developers use local properties often, these properties might add a lot of extra cognitive load when writing new CSS. |
|
@thibaudcolas as per discussion today
I think this kind of thing is a good reason why we want to try to keep more CSS a lintable format (or restricted by what tailwind classes provide) across the application. And also, not provided too much by external libraries. It is hard enough to get our own core styles written in a way that makes it easy to provide RTL, let alone externally supplied CSS. It is easy to reach for position: absolute / float but it will keep causing problems if we want to really support RTL and multiple languages well. |
|
@kaedroho the underlying goal of this is to be able to add future support for RTL See |
thibaudcolas
force-pushed
the
feature/rtl-logical-css
branch
from
2c5d6e0
to
7f0dcc6
Compare
|
@kaedroho as LB mentions, the goal is RTL support. In particular supporting RTL languages with as little "RTL overrides" as possible – the styles should just be written with direction-agnostic CSS techniques. Logical properties and values are the simplest way to do this, and are becoming a solid option now that our browser support targets allow us to use them. We’re only using logical properties/values for RTL mirroring, so there are only 10 or so properties in frequent use, which should be straightforward to get used to (basically |
thibaudcolas
force-pushed
the
feature/rtl-logical-css
branch
from
7f0dcc6
to
8cb3a36
Compare
thibaudcolas
changed the title
|
@fabienheureux @lb- would either of you be ok to review this? So far I have tested this successfully with:
I did basic testing in Arabic as well, which shows promising results, but there is much more to do (too much float usage, lots of directional icons). Before:
After:
|
thibaudcolas
force-pushed
the
feature/rtl-logical-css
branch
from
8cb3a36
to
cab029c
Compare
Manage this branch in SquashTest this branch here: https://thibaudcolasfeaturertl-logical-njxbd.squash.io |
A large step towards RTL support (#7793) – this changes all of our existing styles to use CSS logical properties and values, and sets up linting to prevent future usage of
leftandrightin properties as set up our Stylelint configuration.The stylesheet changes were largely mechanical – this was more complex than expected for a few reasons:
insetpositioning, our browser support targets don’t allow us to fully dropleft/rightpositions.floatsupport for flow-relative values is much poorer than I thought – we should likely stop usingfloatfor layout altogether.The linting changes were made more complex by the same issues!