Skip to content
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

Fix code selection background colors #3396

Merged
merged 4 commits into from
Mar 17, 2025

Conversation

dipamsen
Copy link
Contributor

@dipamsen dipamsen commented Mar 14, 2025

Fixes #1742

Changes:

  • Define background colors to be shown when code is selected (For light and dark theme, they were already defined, but not used properly. For high contrast mode, the background color selected is WCAG AA compliant.)

  • Properly override CodeMirror defaults corresponding to the following CSS styles defined by codemirror.css

    .CodeMirror-selected { background: #d9d9d9; }
    .CodeMirror-focused .CodeMirror-selected { background: #d7d4f0; }
    .CodeMirror-line::selection, .CodeMirror-line > span::selection, .CodeMirror-line > span > span::selection { background: #d7d4f0; }
    .CodeMirror-line::-moz-selection, .CodeMirror-line > span::-moz-selection, .CodeMirror-line > span > span::-moz-selection { background: #d7d4f0; }

Comparison (High Contrast Mode):

Before After
screenshot of selected code before change screenshot of selected code after change

I have verified that this pull request:

  • has no linting errors (npm run lint)
  • has no test errors (npm run test)
  • is from a uniquely-named feature branch and is up to date with the develop branch.
  • is descriptively named and links to an issue number, i.e. Fixes #123

Copy link

welcome bot commented Mar 14, 2025

🎉 Thanks for opening this pull request! Please check out our contributing guidelines if you haven't already.

@Harshit-7373
Copy link
Contributor

@dipamsen I feel that the issue you have solved requires some discussion about how it should look.

@raclim , Please share your thoughts on this.

@dipamsen
Copy link
Contributor Author

dipamsen commented Mar 14, 2025

@Harshit-7373 I have just used the colors that were already defined in the code to be background colors for selection, and used them to override the corresponding CodeMirror CSS properties.

Design wise, I have only chosen one new color (from the predefined color variables) which is the background color for selection in the high contrast theme. I am open to changing it if required, upon further discussion.

@raclim
Copy link
Collaborator

raclim commented Mar 17, 2025

Yes, I think the discussion does illustrate that we probably want the highlight color to be a predefined scss variable that we're already using within the editor. (Pasting in the referenced comment below.)

I also think it makes sense to use one of the variations of gray that are used in the theme rather than strong colors like pink and yellow, as that's more common in the context of text highlight colors.

Since this change is pretty small and contained, I feel like what we have a good idea of what we want the end result to look like, which is to have an appropriate contrast between the text and the highlight. I think this feels okay for me, and looks good in the Dark contrast mode as well. Thanks so much for your work on this!

@raclim raclim added Area:Accessibility Category for accessibility related features and bugs Area:CSS labels Mar 17, 2025
@raclim raclim merged commit bdb00d2 into processing:develop Mar 17, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area:Accessibility Category for accessibility related features and bugs Area:CSS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Highlighting Text with a Mouse Drag Cause Text to Become Unreadable in High Contrast Mode
3 participants