-
-
Notifications
You must be signed in to change notification settings - Fork 851
RiverLea: issue149, restructure/simplify colours to ease stream-making/customiser #33640
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷 Introduction for new contributors...
Quick links for reviewers...
|
a78e6d7 to
94c0414
Compare
57990ad to
31b746e
Compare
|
Ooh, |
|
@colemanw - yeh v cool. I thought I could completely swap hsl() for it but that’s still needed in some places.
But the hacky background-gradient I’m using for buttonhover tints can go (just noticed they’re still in RL so this needs another commit).
|
03b7754 to
087fdd3
Compare
|
@vingle I wonder if CSS can now do what we were previously using javascript for: Lines 1975 to 1983 in 35103b4
|
|
Unfortunately only by describing each colour as separate rgb variables, then doing some calc (https://css-tricks.com/switch-font-color-for-different-backgrounds-with-css/). contrast-color is the dream css for this in the pipeline, but so far only Safari have implemented, so it's a few years away. But maybe that JS could be used in the customiser to auto-select the right text colour variable to sit on top of a button/bg colour and save the user from figuring it out / getting it wrong. |
60c113b to
07515ec
Compare
|
|
||
| .crm-container details { | ||
| background-color: var(--crm-expand-body-bg); | ||
| /* background-color: var(--crm-expand-body-bg); Removed for FormBuilder clashes */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@artfulrobot - in this quite bit refactor of colours and variables names - the relevant bits for Thames - other than changing variable names I've removed a couple of your override bg colours on details and summary here and in your dark.css - as it was rendering illegibly on SearchKit edit screen. But maybe there's somewhere else you have details without crm-accordion-light/dark classes that need this - so flagging.
|
@vingle the merge conflict is between 2 changes of yours it seems: |
|
Thanks @colemanw - I think that's just since the recent SearchKit accordion merge, the other 17 errors remain… |
|
Rebased as #33802 |
https://lab.civicrm.org/extensions/riverlea/-/issues/149
This big change to RiverLea does several things:
color-mix()andhsl(), making it easier for customisers to change one colour value (e.g.--crm-c-success) and have related colours auto-generated (e.g.--crm-c-success-light). This has been done on some background colours, some emphasis colours, and alternating rows in tables / checbox groups.backgroundtobgwhere it's found.An example of the impact of changes: previously if you duplicated a stream and changed
--crm-c-page-backgroundto the background colour you want, a bunch of other parts of the UI would change to that colour too. The parts would vary depending on which Stream you were on - it was neither consistent or predictable. Now--crm-c-page-bgapplies just to the background.The reason so many changes have been done together is testing 4 streams in light and dark modes is time consuming, so it's easiest done all at once.
I have yet to test this PR - and will do this now the changes are complete.
Changes in more detail
crm-c-layer2-bg(crm-c-background3) is auto-generated fromcrm-c-layer1-bg(crm-c-background2).--crm-c-X-light) are now auto-generated.Reduces alternating table row colours to just two - row-bg and row-hover. Auto-generates alternating colours, alternating hover colours, and sort column colours (visible in Extensions manager, no way to do that SearchKit as no classes). NB - this forces the alternating colour to be a fixed tint of the bg colour. If you want alternating red and yellow row colours, this won't help that.
Shortening selector lists with
:is()when a chance is spotted (not comprehensive, just an ongoing process).Darkens checkbox row colours for darkmode Minetta.
Have tried to keep current UI as close to unchanged as possible. If something has changed it's probably a bug. Notable changes are screengrabbed in the first (now closed) PR - #33626.