[RFC/WIP] Refs #????? -- Switched to logical properties in CSS. #17560
Conversation
…ies (4-value shorthand).
…erties (4-value shorthand).
…erties (3-value shorthand).
…erties (2-value shorthand).
…erties (individual).
…ties (4-value shorthand).
…ties (3-value shorthand).
…perties (4-value shorthand).
…perties (3-value shorthand).
…perties (2-value shorthand).
…perties (individual).
…roperties (4-value shorthand).
…h properties (3-value shorthand).
…h properties (2-value shorthand).
…h properties (individual).
…roperties (4-value shorthand).
…r properties (3-value shorthand).
…r properties (individual).
…thand properties.
…text-align property.
There was a problem hiding this comment.
Some (very) initial thoughts:
- Great idea!
- I wonder how much of this code is used or necessary as we've moved more and more to flex.
- I didn't know these existed at all! I worry a bit that these selectors are less well known and a bit harder for people to immediately understand.
From 2:
- Is it better to do an audit and clean up unused styles first or do this first? Or no substantial difference?
- I'd like to move more things to flex (it's on my list...), which would also likely reduce the need for RTL-specific styles.
Why, thank you! 😄
A good question, and probably not straightforward to answer. I could have a look through to see if there are any unused rules as a first pass. This will still probably make migrating things over to flex layout easier as it'll reduce how much thought needs to go into margins/padding, etc. There will also be cases that will still require these properties over flex. In addition, logical properties also help for vertical layout modes rather than only thinking about RTL.
They've been around for quite a while, but some have taken more time to mature and there are still some issues, notably around shorthand rules, e.g. This is also why I proposed a simple linting rule to check that we're not using the undesired properties.
Probably worthwhile, yes. I already did some partial clean up in b9ad9fe and would likely look at what using CSS variables and
For sure. As mentioned above, I think this could simplify that migration to more flex stuff by already cleaning up some of the warts related to RTL. Now that we also have the support for generating screenshots in CI, it'll be interesting to do some comparisons! 🖼️ |
|
This is great :) The need for this is pretty clear-cut to me, it’s just not productive to maintain separate RTL styles when logical properties and values are possible, but there are enough specific implementation details that need careful consideration so I’d recommend proceeding with a DEP so there’s room for people to validate specific details of the implementation plan. I’d like to see specifically:
If possible – I don’t think Django does this well currently but I’d also like to see:
And ideally as "stretch goals":
I wouldn’t recommend the cleanup being a pre-condition for this work. There’s plenty enough benefits to switching even unused code to logical properties and values :) |
Ok. Not sure there is all that much that it would require a DEP, but I guess we can look into doing one... Has the when to DEP or not thing been worked out yet - I know there was some discussion recently? It's never really been clear, and I'm concerned I'd spend time on something that nobody will engage with.
So currently it looks like we're at 82.92% for the logical float/clear stuff. Am sure this has jumped up since last week. Based on our browser support, there is some leeway with this, but I expect we wouldn't be looking at landing something like this for a few months yet, but we can still plan.
The point is that the vertical stuff should largely come for free (except for the
For sure. We'd want to make sure we don't regress. This is quite trivial, but we definitely will need something in place. Have seen the discussions and it seems positive, which is great.
This will be the biggest thing. Linting will help highlight stuff, but we effectively need to define our policy on this so that it's clear what the expectation is when contributing. Alas, visibility of this always seems to be the challenge - people will always contribute without reading the guidelines 🤷🏻 When we do this, we'll likely call it out in the top section of the release notes that the admin has changed to using logical properties with a link to the details in the conributing guide. The good news is that, where people have overridden stuff using physical properties, they've probably largely only considered LTR anyway, and things should mostly continue to work as before in those situations with no backward compatibility issues. Even where people have overridden for RTL with physical properties should work in general, but there may be small issues here and there.
This is what I want to investigate next. We'll need one of those overrides at the top level to define the variable(s), but the idea is that everything else flows from that.
A good shout. I don't think there is much, but this reminds me that we should also look at the various error pages, etc. I've mainly focused on the admin at this point.
Also interesting. Some of this is handled by using multiple icons, but are there situations where we can flip and/or rotate instead? 🤔
Another excellent point. I have an idea of a few people we could ask, but it'd be good to put out a call on the forum, perhaps when we have progressed with an initial DEP or such...
While I'm all for this, I think we should keep this firmly as a separate goal. I don't see that switching properties from physical to logical would be an impediment to this in any way - in fact, I think it may even simplify the task. It's likely that we can even delete some of the logical styles when migrating things to flex or grid as those layout models take flow directions into consideration by design.
We've added the capability to take screenshots, but there isn't support for easy comparison yet. I guess we could look into something like percy.io or equivalent - maybe some generous company will donate it as a freebie? 🤞🏻
Agreed. Clean up is good irrespective of when it comes. If some things come before, that's just a bonus of smaller diffs for review. |
|
@ngnpope if you’re hesitant to start the DEP I’d happily volunteer to do that, at least a first draft which you can add to at a later date? Let me know what you think. @knyghty what do you think, does this "require a DEP"? As far as engaging with it once opened – there are a lot of glaring issues with Django’s RTL support currently, and @django/accessibility has been struggling a lot with the state of Django’s CSS, which this work would help in improving quite a bit. So personally I’d suspect this will be smooth-sailing to take through. Re browser support – sounds good to me!
It’s not really for free though, as it means people who write code will need to think of left-right mirroring and of the "vertical" axis direction. Personally I doubt anyone wants to use the Django admin in a TTB-RTL layout, so I’d rather we skipped this entirely.
I would recommend it’d be better for maintenance if we had a policy to only use icons in the UI that are either non-directional, or that can safely be mirrored. That way you don’t have to write so many overrides on "which icon to use in which variant of the admin" – you just write individual icon definitions with the LTR perspective, and then have some way to define which icons are and aren’t mirrored. A note on icons mirroring – I think this should also be in the "separate goal" / "not MVP" area, because Django’s icon definitions are currently really hard to work with for other reasons, and I think it’d be better to fix this tech debt before changing the code for RTL support. Starting to use logical properties and values is plenty enough work as it is.
Same. I think it’d be nice to cover this in the DEP nonetheless, but not as something that must happen as part of this switch to logical properties.
Yeah Percy would be great. It does have a free tier, but I’m not sure the free tier would be enough for what we’d need here. I can recommend BackstopJS for one-off efforts like this, as something we’d run locally. I’ve used it extensively so happy to expand further on what this would look like. |
|
There’s overwhelming support for setting up Prettier for CSS in the forum discussion about Adding a formatter for CSS & JS. Based on discussions here and on that forum thread, do people think we should start a forum discussion about linting CSS already? Or wait for more to happen with either the auto-formatting or this logical properties and values refactoring? |
|
I think it's worth starting the discussion. Things can be done in phases - just having something in place to begin with would be great and then adapting the rules for logical properties later would be best. |
|
Regarding the DEP - sounds fine if there is enough interest in it. Happy to put something together if we want to take that path. I just wasn't sure whether this was big enough a change to warrant it.
Nice. I'm also currently working on overhauling |
I think it's easier to start with a forum post and then move to a DEP if there's no clear consensus, personally. But it's also unclear on what requires one or not. |
|
👍 thanks! Main reason I suggested a DEP over a forum post is there there are a lot of aspects to this that I think it’d be easier to validate by reviewing a documented implementation plan, where there can be multiple drafts, rather than a long thread of replies. But if others think it’s not needed, happy to follow suite! |
|
Hi @ngnpope, happy new year :) I thought I’d check in and ask what your plans are to move this along? I see right-to-left support as a great way to get new people involved with core development, as there are lots of obvious gaps. I’m aware of a few development sprints coming up in the first half of 2024, as well as programs dedicated to new contributors – where this work would make for lovely contributions. What are your plans? Do you still intend to publish a forum post about this? |
|
Hi @thibaudcolas. I haven't really had a chance to look into this yet. Might do later in the month. |
|
I think there's no need to rush here anyway... I get that it'd be a nice clean up prior to flex stuff, but if you want to focus on that, then I think that should happen independently.
This is now 85.57% with Chrome < 118 at 5.28% being the biggest detriment. In a few months time I expect we'll be > 90% and it'll mostly be mobile browsers, e.g. Samsung Internet, that affect this. This will be one of the things that I want to consider - do we need to go down the
I think I'll do a forum post, but I'd like to consider some more things first, e.g. an approach to handling background images and icons. |
|
👍 thanks :) Samsung Internet is a Chromium derivative, I don’t know how many versions behind Google Chrome they’re at but we could do the math and I wouldn’t be surprised if they did land support by the time this work would be released for Django. For Opera Mobile, be worth double checking Django’s exact browser support rules but I wouldn’t expect it’s one we want to support. |
Over the past few months there have been a lot of regressions and subsequent fixes for various issues with RTL CSS.
For example: #17561, #17558, #17556, #17516 (comment), #17514, #16685, #16684, #16675... And that's just this year alone.
I'm wondering if it's just time to switch as much as possible of this over from physical to logical properties?
Support is generally pretty good now. We're probably just waiting a few months for the support percentage to creep up for Chrome/Edge for
float,clearand some others which were unflagged in v118, but I think it's worth discussing now. We can also make use of@supportsif necessary -- which is widely supported -- to check for some cases and fall back to physical properties for a period of time. A notable exception is lack of support for logicalbackground-position, but we could perhaps be clever with use of CSS variables for this (which I haven't investigated here).I've knocked up a script that can generate a bunch of commits so that we can easily re-run it. There will likely be a few manual things to tweak also, e.g. related to splitting shorthand properties that don't support logical dimensions. We'd also need to ensure that the documentation is updated to require use of logical properties (and perhaps add a linting check to avoid them creeping in).
Anyway... Is this a good idea worth pursuing? Pinging @knyghty and @felixxm for their input as they've fixed most of these issues.