Skip to content

RiverLea 1.3.8: 14 dark-mode fixes, 3 other fixes, 2 new variables & #31994 - #32209

Merged
ufundo merged 1 commit into
civicrm:6.0from
vingle:6.0
Feb 26, 2025
Merged

RiverLea 1.3.8: 14 dark-mode fixes, 3 other fixes, 2 new variables & #31994#32209
ufundo merged 1 commit into
civicrm:6.0from
vingle:6.0

Conversation

@vingle

@vingle vingle commented Feb 25, 2025

Copy link
Copy Markdown
Contributor

Sorry for a large PR into the RC branch - but a chunk of this is @ufundo's PR, and of the other 17 changes - 14 impact dark mode. They're fixes to things that are illegible/non-functional - so the changes seem necessary. They were only found while doing a slow detailed test across all streams of #31994 - and that's why they're bundled together.

1. Riverlea - merge crm-c-light-text and crm-c-text-light

This includes the PR #31994. If that gets merged first then this needs rebasing. Related to that is the addition of two new CSS variables:
--crm-checkbox-list-bg:
--crm-checkbox-list-bg2:

2. Bootstrap Time input fields have a sensible width, not 100%

As seen in SearchKit Configure Settings tab. This is perhaps the one PR that's not involving broken

Before

image

After

image

3. PrettyPrint code blocks (e.g. on API3 & API4) should wrap when in limited space.

NB - this does fix the issue in all code blocks but many

Before

image

After

image

4. Contact Dashboard inline edit name - reset position and add drop-shadow to distinguish

This fixes a regression with the position of inline-edit on the contact dashboard, possibly linked to this. It impacts all Streams a little differently.

Before

image
image
image
image

After

image
image
image
image

5. All Dark modes - forces a light bg colour to 'prettyprint' code blocks as inverting the colours would require many replacements, also addresses some API3 code blocks.

Before

image

After

image

6. All Dark modes - FormBuilder input text illegible, changed colour to 'crm-c-text'

Before

image

After

image
image

7. ALL Dark modes - FomrBuilder GUI bar inline span colour illegible, changed colour to 'crm-c-text'

Before

image

After

image
image

8. All Dark - Select2 'disabled' list items illegible, set bg colour and changed cursor icon to 'not-allowed'.

Before

image

After (can't show cursor in screengrab)

image
image

9. All Dark - FormBuilder crm-ui-editable region hover illegible, changed text colour from inherit to 'crm-c-text-dark' - created RL version of ang/crmUI.css to achieve.

Before

image
image

After

image
image
image

10. ALL Dark - FormBuilder settings / gears icon was an illegible colour.

Before

image

After

image

11. All Dark - SearchKit 'where' 'and' labels illegible. Changed colour to 'crm-primary-text' to match 'crm-primary' background.

NB - for reasons not immediately clear, the #fff text is tinted, so not WCAG AAA - but that could be tackled in 6.1.

Before

image

After

image

12. All Dark - pie chart legend text made legible, setting text fill colour to 'crm-c-text'

Before

image

After

image

13. All Dark - '.alert-warning' paragraph text given explicit colour to resolve clash with '.alert' paragraph colour.

Before

image

After

image

14. Minetta & Hackney Dark illegible info alerts: 'crm-alert-text-info' changed to '-—crm-c-text-light' from '-—crm-c-blue-light'

Before

image

After

image

15. Minetta & Hackney Dark - illegible alert buttons: removed '--crm-c-alert-text' & '-—crm-c-alert' from dark.css

Before

image

After

image

16. Hackney & Thames Dark - warning alert text colour setting removed to make legible

This is likely a regression from 31994.

Before

image

After

image

17. Minetta Dark - primary/primary hover darkened with dark.css variables to stand out on tab region navbar.

Before

image

After

image
image

18. Walbrook Dark - Crm-c-success-text & crm-c-warning-text - change from dark to light as bg colours have changed.

This is likely a regression from 31994.

Before

image

After

image

Technical Details

This has been tested across all four streams, dark and light modes: on ThemeTest, FormBuilder & Search UIs, Contact Dashboard - all tabs & edit, Civi Dashboard, API3, API4.

Comments

Sidenote while doing this with a new contrast ratio tool I noticed that not every screen is 100% using WCAG AA or AAA contrast ratios, and there's also legibility issue with dark-mode code layout in the API4 Explorer Code Panel help box. However I felt both of these points could be tackled on master branch against 6.1/6.2, while these issues here impact functionality (in dark mode).

@civibot

civibot Bot commented Feb 25, 2025

Copy link
Copy Markdown

🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷

Introduction for new contributors...
  • If this is your first PR, an admin will greenlight automated testing with the command ok to test or add to whitelist.
  • A series of tests will automatically run. You can see the results at the bottom of this page (if there are any problems, it will include a link to see what went wrong).
  • A demo site will be built where anyone can try out a version of CiviCRM that includes your changes.
  • If this process needs to be repeated, an admin will issue the command test this please to rerun tests and build a new demo site.
  • Before this PR can be merged, it needs to be reviewed. Please keep in mind that reviewers are volunteers, and their response time can vary from a few hours to a few weeks depending on their availability and their knowledge of this particular part of CiviCRM.
  • A great way to speed up this process is to "trade reviews" with someone - find an open PR that you feel able to review, and leave a comment like "I'm reviewing this now, could you please review mine?" (include a link to yours). You don't have to wait for a response to get started (and you don't have to stop at one!) the more you review, the faster this process goes for everyone 😄
  • To ensure that you are credited properly in the final release notes, please add yourself to contributor-key.yml
  • For more information about contributing, see CONTRIBUTING.md.
Quick links for reviewers...

➡️ Online demo of this PR 🔗

@civibot civibot Bot added the 6.0 label Feb 25, 2025
@colemanw

Copy link
Copy Markdown
Member

@vingle there's a stylechecker fail

@vingle

vingle commented Feb 25, 2025

Copy link
Copy Markdown
Contributor Author

Thanks! updated…

Comment on lines -256 to -258
--crm-alert-background-warning: var(--crm-c-yellow-light);
--crm-alert-border-warning: var(--crm-c-yellow);
--crm-alert-text-warning: var(--crm-c-text);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@artfulrobot there's a bunch of changes to Thames in this PR, but all of them are Ufundo's #31994 and are structural with the exception of here - which is trying to fix this problem below (somewhat linked to light/dark text). However I just realise I made a mistake - will commit a fix…

image

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks fine in thames to me now:

image

@ufundo

ufundo commented Feb 26, 2025

Copy link
Copy Markdown
Contributor

Looks good to me too @vingle

All the pain points I'm aware of look good to me and definitely good to have those variable names sorted.

Thanks for all your testing and you too @artfulrobot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants