Improved placeholders and font color grids in high contrast mode #16284
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…oot and nested editables.
WalkthroughThe update aims to improve accessibility features in CKEditor components, especially for high-contrast mode compatibility. It introduces mixins for media colors, adjusts color handling in UI components, and enhances visibility in various display settings. Changes
Assessment against linked issues
Recent Review DetailsConfiguration used: CodeRabbit UI Files selected for processing (5)
Files not reviewed due to errors (4)
Additional comments not posted (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
scofalik
approved these changes
Apr 29, 2024
| height: var(--ck-color-grid-tile-size); | ||
| min-width: var(--ck-color-grid-tile-size); | ||
| min-height: var(--ck-color-grid-tile-size); | ||
| padding: 0; | ||
| transition: .2s ease box-shadow; |
There was a problem hiding this comment.
I understand that this applies to both normal and contrast mode. Do we want to keep transitions for contrast mode?
There was a problem hiding this comment.
I think these two domains of UX are orthogonal. I see nothing wrong with leaving it as it is.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Comment on lines
+26
to
+30
| /* Improve placeholder rendering in high-constrast mode (https://github.com/ckeditor/ckeditor5/issues/14907). */ | ||
| @mixin ck-media-forced-colors { | ||
| background-color: unset; | ||
| color: unset; | ||
| } |
There was a problem hiding this comment.
Incorrect usage of mixin syntax in plain CSS.
- @mixin ck-media-forced-colors {
+ @include ck-media-forced-colors;Ensure that the project is using a CSS pre-processor that supports mixins, such as SASS, or adjust the approach to comply with plain CSS standards.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Suggested change
| /* Improve placeholder rendering in high-constrast mode (https://github.com/ckeditor/ckeditor5/issues/14907). */ | |
| @mixin ck-media-forced-colors { | |
| background-color: unset; | |
| color: unset; | |
| } | |
| /* Improve placeholder rendering in high-constrast mode (https://github.com/ckeditor/ckeditor5/issues/14907). */ | |
| @include ck-media-forced-colors; | |
| background-color: unset; | |
| color: unset; | |
| } |
Comment on lines
+35
to
+38
| @mixin ck-media-default-colors { | ||
| &.table__caption_highlighted { | ||
| animation: ck-table-caption-highlight .6s ease-out; | ||
| } |
There was a problem hiding this comment.
Incorrect usage of mixin syntax in plain CSS.
- @mixin ck-media-default-colors {
+ @include ck-media-default-colors;Ensure that the project is using a CSS pre-processor that supports mixins, such as SASS, or adjust the approach to comply with plain CSS standards.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Suggested change
| @mixin ck-media-default-colors { | |
| &.table__caption_highlighted { | |
| animation: ck-table-caption-highlight .6s ease-out; | |
| } | |
| @include ck-media-default-colors; | |
| &.table__caption_highlighted { | |
| animation: ck-table-caption-highlight .6s ease-out; | |
| } |
earthtodani
added a commit
to AuthoriumDPA/ckeditor5-authorium
that referenced
this pull request
May 30, 2024
* Docs: minor fixes. [short flow] * Shorten the clipboard guide. * Docs: formatting. [skip ci] * Docs: timestamp bump. [skip ci] * Docs: Unify the usage of quotation marks in the React guide. * Docs: Update the Vue guide to new installation methods. * Docs: Fix indentations in the React guide * UI animations do not respect user's preferences on reduced motion (ckeditor#16099) Feature: Editor UI should respect the user's preferences for reduced motion (WCAG 2.1, Success Criterion 2.3.3). Feature (utils): Implemented the `env#isMotionReduced` flag to discover reduced motion preferences. * Improved placeholders and font color grids in high contrast mode (ckeditor#16284) Feature (utils): Implemented the `env#isMediaForcedColors` property for forced colors detection (e.g. high contrast mode on Windows). See ckeditor#14907. Feature (ui): Implemented `ck-media-forced-colors` and `ck-media-default-colors` mixins for detecting forced colors (e.g. high contrast mode on Windows). See ckeditor#14907. Fix (theme-lark): The caret should be visible in a placeholder while in forced colors mode (e.g. high contrast mode on Windows). Improved the look of the placeholders in the forced colors mode. Closes ckeditor#14907. Fix (theme-lark): The color grid component should render as a grid of labels in the forced colors mode (e.g. high contrast mode on Windows). Closes ckeditor#14907. * Docs: import link fix. [short flow] * Use `dev-utils` with updated esbuild-loader * Fix issues caused by targeting ES2022 (and dropping some polyfills) * Image jumps when resizing in container that has padding (ckeditor#16304) Fix (image, widget): An image should not jump upon resizing in a container with padding. Closes ckeditor#14698 --------- Co-authored-by: Aleksander Nowodzinski <a.nowodzinski@cksource.com> * Name, role, value accessibility fixes (ckeditor#16295) Fix (restricted-editing): Improved the accessibility of the restricted editing dropdown by setting the correct ARIA role on the toolbar menu. Fix (minimap): Addressed the issue of the minimap `<iframe>` being unnecessarily exposed to assistive technologies. --------- Co-authored-by: Aleksander Nowodzinski <a.nowodzinski@cksource.com> * Add menu bar integration to multi root editor. (ckeditor#16280) Feature (editor-multi-root): Added menu bar support for multi-root editor. * Other: Updated translations. [skip ci] * Other: Updated translations. [skip ci] * Internal (build-*): Builds. * Add widget ESC shortuct accessibility dialog entry (ckeditor#16310) Other (widget): Added information to the Accessibility help dialog about a keyboard shortcut that moves focus from a nested editable back to the widget. --------- Co-authored-by: Aleksander Nowodzinski <a.nowodzinski@cksource.com> * Docs: Update the Next.js guide to new installation methods. * Docs: Update builder links in integrations. * No longer delay showing tooltip while focusing an element (ckeditor#16296) Fix (ui): `TooltipManager` tooltips should immediately show up when triggered by user focus for better responsiveness and accessibility. --------- Co-authored-by: Aleksander Nowodzinski <a.nowodzinski@cksource.com> * Mark mixin calls as pure * Add `/* #__PURE__ */` before mixin calls in global scope * Other: Updated translations. [skip ci] * Internal (build-*): Builds. * Docs: update Builder links. [short flow] * Docs: update Builder links. [short flow] * Use regular `dev-utils` * Bump `ckeditor5-dev-*` packages to `^40.0.0` * Use `declare` for `View.viewUid` property * Updated the Accessibility guide with VPAT for the latest editor version and new keystrokes added to the list (ckeditor#16324) Docs: Updated the Accessibility guide with VPAT for the latest editor version and new keystrokes added to the list. * Docs: testing helpers fix. [short flow] * Remove menu bar on editor destroy. (ckeditor#16329) Tests (editor-multi-root): Removing the menu bar element on editor destroy. * Docs: minor update. [skip ci] * Docs: minor update. [skip ci] * Add `declare` to few dynamically populated fields * Docs: lists install update. [short flow] * Docs: UI language link. [short flow] * Docs: API links. [short flow] * Docs: API links. [short flow] * Docs: restoring new imports. [short flow] * Update docs/tutorials/widgets/data-from-external-source.md Co-authored-by: Piotr Szczęśniak <szczesniakp@o2.pl> * Update docs/tutorials/widgets/implementing-a-block-widget.md Co-authored-by: Piotr Szczęśniak <szczesniakp@o2.pl> * Update docs/tutorials/widgets/implementing-a-block-widget.md Co-authored-by: Piotr Szczęśniak <szczesniakp@o2.pl> * Update docs/tutorials/widgets/implementing-an-inline-widget.md Co-authored-by: Piotr Szczęśniak <szczesniakp@o2.pl> * Docs: Updated the 2.5.7 section of the VPAT document. * Internal: Move the internal `generatePositions` function to a static method on `BalloonPanelView` class. * Docs: lotsa various fixes. [short flow] * Docs: Fix link to `BalloonPanelView.generatePositions` * Docs: lotsa various fixes. [short flow] * Docs: lotsa various fixes. [short flow] * Docs: Updated the 4.1.3 section of the VPAT document. * Docs: expanding snippets. [short flow] * Docs: crosslinking. [short flow] * Increased specificity of image-style-align CSS classes. * Internal: Vale evaluation. [skip ci] * Docs: post-tests fixes. [short flow] * Docs: Changelog. [skip ci] * Docs: language fixes. [short flow] * Docs: Corrected the changelog. [skip ci] * Docs: Added an intro section. [skip ci] * Docs: explaining Essentials. [short flow] * Docs: minor rearrangements. [short flow] * Increased specificity of almost all image-style-align CSS classes. * Docs: updating and examples update. [short flow] * Docs: await update. [short flow] * Docs: setup rearrangements. [short flow] * Docs: minor fix. [skip ci] * Docs: Fix typos in the React guide. * Docs: Update the Angular guide to new installation methods. * Docs: review fixes and more changes. [short flow] * Docs: removing the removing features section. [short flow] * Docs: minor technical fix. [short flow] * Docs: removing infoboxes. [short flow] * Docs: removing redundant guides. [short flow] * Docs: restoring the removal section. [short flow] * Docs: restoring an internal link. [short flow] * Changelog review. * Docs: Prepare a structure for the Vue 2 guide. * Docs: Unify subhedings in integrations. * Docs: unsupported packages infobox. * Docs: typos. [short flow] * Docs: quick start update. [short flow] * Docs: quick start update. [short flow] * Docs: quick start update. [short flow] * Docs: updating UI language guide. [short flow] * Update ui on close image ckbox dialog. * Docs: updste. [short flow] * Docs: updste. [short flow] * Docs: updste. [short flow] * Internal: Improve tree-shaking by adding `#__PURE__` comments before function calls in global scope * Update docs/getting-started/setup/ui-language.md Co-authored-by: Filip Sobol <filipsobol@users.noreply.github.com> * Docs: update. [short flow] * Docs: updating Builder link. [short flow] * Docs: updating API install commands. [short flow] * Docs: updating API install commands. [short flow] * Release: v41.4.0. * Docs: Add the component installation step to the Vue 3 guide. * Docs: Fix import in the Vue snippet. * Docs: Fix formatting in the Vue guide. * Docs: Add context to the Vue guide. * Docs: Update snippets in the Vue 3 guide. * Docs: Change the Angular guide structure. * Docs: Unify collaboration sections across integrations. * Docs: Fix typo. * Docs: Add the localization section to the Vue 2 guide. * Docs: Unify the localization section in Vue integrations. * The TooltipManager should verify if the tooltip is not already unpinned while trying to update its position. * Removed unnecessary non-null assertions. * Updated the GH link to point to the public repo. * Added info about the changes related to the silent release of NIM. See: ckeditor#16360 (comment) * Merge pull request ckeditor#16363 from ckeditor/cc/6219-update-race Fix (ui): Prevented editor error in a situation where tooltip was unpinned after it was already removed. This happened when "Unlink" button was pressed while the tooltip was shown. * Internal (build-*): Builds. * Docs: Changelog. [skip ci] * Docs: Changelog. [skip ci] * Internal: An empty commit to trigger the release. * Docs: Changelog. * Release: v41.4.1. * Docs: new methods READMEs. * Merge master. * Internal: Fixed wording in the `ImageResize` plugin description in `ckeditor5-metadata.json`. [skip ci] * Docs: updating main README. [short flow] * Docs: adding decoupled editor toolbar config. [short flow] * Add clearTimeout and test. * Docs: a typo. [skip ci] * Update docs/getting-started/setup/toolbar.md Co-authored-by: Piotr Szczęśniak <szczesniakp@o2.pl> * Docs: snippet update. [short flow] * Docs: Update the Laravel integration guide to new installation methods. * Added image style align left/right between image and image-inline. * Fix (utils): Prevented error thrown when `ckeditor5-utils/src/env.ts` is used in an environment without `window` global object. * Fixed lint issue. * Rename `globalVar` to `global`. * Docs (utils): Added a clarification remark to some properties in `env` object. * Docs (utils): Grammar and typo fix. * Internal (build-*): Builds. * Docs: Changelog. [skip ci] * Docs: Changelog (v41.4.2). * Release: v41.4.2. * Docs: removing builds. [short flow] * Docs: Unify the setup sections. * Docs: updates. [short flow] * Docs: Prepare a structure for the .NET integration. * Docs: Fix typo. * Docs: Update the .NET integration guide to new installation methods. * Docs: Fix contraction. * Other: Add `#__PURE__` comments before objects and arrays in global scope. * Internal: Vale evaluation. [skip ci] * Docs: updating VPAT. [short flow] * Instant pin tooltip on focus only if not hovered * Docs: updating Drupal guide. [short flow] * Docs: restoring a deleted file. [short flow] * Add `onAfterDestroy` callback to react docs snippet, minor wording changes * Docs: Minor fixes in the Quick Start * Docs: Minor fixes in the Angular integration * Docs: Minor fixes in the Next.js integration. * Docs: Minor fixes in the Vue 2 integration. * Docs: Minor fixes in the Vue integration. * Docs: Modify the structure of the Next integration. * Docs: Add new method to the React integration. * Docs: minor fixes. [short flow] * Core review fixes. * Docs: puntuation. [short flow] * Tweak plugins. * Change spinner animation keyframe name to be more unique. * Fix theme and CSS. * Fix watchdog setup. * Docs: general fixes. [skip ci] * Docs: general fixes. [skip ci] * Docs: relinking and cleanup. [short flow] * Code review fixes. * Docs: Unify the Builder naming. * Code review fixes. * Remove type. * Docs: Remove excessive articles. * Docs: Remove the excessive section from the Angular guide. * Docs: Preliminary work in the Block widget guide. * Docs: updating error codes. [short flow] * Docs: updating error codes. [short flow] * Docs: Update the Block widget tutorial to new installation methods. * Docs: Fix typos in the Block widget guide. * Docs: minor fixes. [short flow] * Docs: pre-release fixes. * Docs: integrations update. [short flow] * Docs: adding CTA. [short flow] * Docs: Preliminary work in the Inline widget tutorial. * Docs: minor update. [short flow] * Docs: minor update. [short flow] * Docs: minor update. [short flow] * Docs: Update the Inline widget tutorial to new installation methods. * Docs: a review. [short flow] * Docs: Builder infoboxes. [short flow] --------- Co-authored-by: godai78 <b.biedrzycki@cksource.com> Co-authored-by: Witek Socha <witek.soch@gmail.com> Co-authored-by: Bartek Biedrzycki <68123541+godai78@users.noreply.github.com> Co-authored-by: Mateusz Gorzeliński <mateusz.gorzelinski@gmail.com> Co-authored-by: Mateusz Bagiński <cziken58@gmail.com> Co-authored-by: Aleksander Nowodzinski <a.nowodzinski@cksource.com> Co-authored-by: Filip Sobol <filipsobol@users.noreply.github.com> Co-authored-by: Michał Remiszewski <118178939+mremiszewski@users.noreply.github.com> Co-authored-by: Kamil Piechaczek <pomek@users.noreply.github.com> Co-authored-by: Piotr Szczęśniak <szczesniakp@o2.pl> Co-authored-by: CKEditorBot <ckeditor-bot@cksource.com> Co-authored-by: Illia Sheremetov <i.sheremetov@cksource.com> Co-authored-by: Kuba Niegowski <j.niegowski@cksource.com> Co-authored-by: Szymon Cofalik <s.cofalik@cksource.com> Co-authored-by: Piotrek Koszuliński <pkoszulinski@gmail.com> Co-authored-by: Witek Socha <w.socha@cksource.com> Co-authored-by: Paweł Smyrek <p.smyrek@cksource.com> Co-authored-by: Kuba Niegowski <1232187+niegowski@users.noreply.github.com> Co-authored-by: Daniela Soto <dsoto@eagerworks.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Suggested merge commit message (convention)
Feature (utils): Implemented the
env#isMediaForcedColorsproperty for forced colors detection (e.g. high contrast mode on Windows). See #14907.Feature (ui): Implemented
ck-media-forced-colorsandck-media-default-colorsmixins for detecting the forced colors (e.g. high contrast mode on Windows) mode. See #14907.Fix (theme-lark): The caret should be visible in a placeholder while in forced colors mode (e.g. high contrast mode on Windows). Improved the look of the placeholders in the forced colors mode. Closes #14907.
Fix (theme-lark): The color grid component should render as a grid of labels in the forced colors mode (e.g. high contrast mode on Windows). Closes #14907.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Documentation