#26315 closed defect (bug) (fixed)
Grunt: RTL & CSSMin Color schemes
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 3.8 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Build/Test Tools | Keywords: | |
Focuses: | Cc: |
Description (last modified by )
Currently Colour schemes don't get RTL'd versions, which results in a very bland UI when using a non-default colour scheme in an RTL environment.
In addition, the colour schemes include @import url("../../colors-fresh.css");
which needs to dynamically change to -rtl.css
so that minification includes the right file
Attachments (4)
Change History (16)
#2
@
11 years ago
Created an upstream PR for grunt-cssjanus
to add a processContent
callback: https://github.com/yoavf/grunt-cssjanus/pull/2
26315.2.diff makes use of it and fixes my earlier issues with the generated files including too much content.
#3
@
11 years ago
Upon further thinking here.. It might be best to make all colour schemes not require a RTL variant at all.
This would mean removing cases like border-left: 4px solid #7ad03a;
and replacing it with border-left-color: #7ad03a
, and adding the appropriate border-left:
declaration in the standard stylesheets, box-shadow:
could cause a bit of pain though, as will gradients.
This has several benefits, the primary one being that 3rd party colour schemes will not have to include .rtl variants and we don't need to make significant changes to our existing colour stylesheet code.
In the event that we DO require 2 different colour borders on an element, we'd have to use .rtl
ourselves in the stylesheet instead.
#4
@
11 years ago
The more I look at not needing a -rtl variant of stylesheets and using .rtl
instead, the more I realise one is needed, there's just too many styles to do it manually it seems.
3rd party colour schemes are going to have a seriously hard time trying to support RTL though, especially as they can't rely on @import like the core styles are using.
#5
@
11 years ago
The upstream change to cssjanus has a typo and doesn't work.
Does this patch work otherwise? (Does need updating.)
#6
@
11 years ago
The upstream change to cssjanus has a typo and doesn't work.
Correct, Looks like out of date code ended up in my commit branch. As long as it doesn't cause a JS error, that parameter is not needed for this ticket.
#9
@
11 years ago
- Owner set to nacin
- Resolution set to fixed
- Status changed from new to closed
In 26616:
#10
@
11 years ago
Running grunt cssjanus:colors produces files like this, if a -rtl.css styles exist already:
File "build/wp-admin/css/colors/blue/colors-rtl-rtl-rtl.css" created.
File "build/wp-admin/css/colors/blue/colors-rtl-rtl.css" created.
File "build/wp-admin/css/colors/blue/colors-rtl.css" created.
File "build/wp-admin/css/colors/light/colors-rtl-rtl-rtl.css" created.
File "build/wp-admin/css/colors/light/colors-rtl-rtl.css" created.
File "build/wp-admin/css/colors/light/colors-rtl.css" created.
File "build/wp-admin/css/colors/midnight/colors-rtl-rtl-rtl.css" created.
File "build/wp-admin/css/colors/midnight/colors-rtl-rtl.css" created.
File "build/wp-admin/css/colors/midnight/colors-rtl.css" created.
Doesn't happen when runnung cssjanus:core multiple times.
It looks like we'll need to add a
processContent
-like callback to the cssjanus grunt module to alter the include, or maybe a option to cssjanus to RTL'ise all include filenames it runs into.I'm attaching my attempt at doing this, but it's not working. I'm not sure why, but
color-schemes/*/colors-rtl.min.css
appears to contain too much data, butcolors-rtl.css
andcolors.min.css
are fine.Obviously, the -rtl.min includes the .min instead of the RTL version still since I couldn't figure out a way to process the content without altering something other than the grunt file.
I don't actually know what I'm doing with Grunt here.. so feel free to ignore the patch :)