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

XWIKI-22746: Images do not have an aria-label in the xwiki-slash dropdown for CKEditor #3784

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Sereza7
Copy link
Contributor

@Sereza7 Sereza7 commented Dec 30, 2024

Jira URL

https://jira.xwiki.org/browse/XWIKI-22746

Changes

Description

  • Added some style to hide "empty" HTML elements for icons.

Clarifications

  • On my test instance that was a bit of an older 17.0.0 snapshot, I couldn't reproduce this. This is because the images were already hidden by a SSX from EditSheet. This style was removed on the 19th of December in 1727219 .
  • I added it with the slash plugin since this plugin .js contains fixes that are generic for all autocomplete uses.
  • When trying to find a proper selector, I took into account the three places we use autocomplete in: xwiki-slash, xwiki-link and xwiki-images. They have different ItemTemplates (we should probably update them to be a bit more consistent) but the selectors used should fix all three.
  • In order to solve the issue, only the first selector is needed. However I added the others so that we don't run into a very similar issue later on (e.g. when updating the item templates in some ckeditor plugin that uses ckeditor autocomplete).

Screenshots & Video

None, no visual changes.

Executed Tests

I successfully built the changes with mvn clean install -f xwiki-platform-core/xwiki-platform-ckeditor/xwiki-platform-ckeditor-plugins ;mvn clean install -f xwiki-platform-core/xwiki-platform-ckeditor/xwiki-platform-ckeditor-webjar/.
After bringing the updated jars in my local distrib, I was able to test the changes out, and the style applied as expected.
I ran mvn clean install -f xwiki-platform-core/xwiki-platform-ckeditor/xwiki-platform-ckeditor-test/xwiki-platform-ckeditor-test-docker -Dgradle.cache.local.enabled=false -Dgradle.cache.remote.enabled=false -Dit.test=LocalizationIT#paragraphQuickAction_en -Dxwiki.test.ui.wcag=true both with and without the changes. I could reproduce the issue reported by CI without the changes, and after the change there was no WCAG fail anymore.

Expected merging strategy

  • Prefers squash: Yes
  • Backport on branches:
    • None

Reasons:

  • This is most likely caused by 1727219 , which was not backported at all AFAICS

…down for CKEditor

* Added some style to hide "empty" HTML elements for icons.

(cherry picked from commit 5f6c494)
Copy link
Member

@mflorea mflorea left a comment

Choose a reason for hiding this comment

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

Looks good. Sorry for causing this issue (I thought the removed CSS styles were needed only for IE11).

This is especially important for assistive tech users (regular users cannot see the image if there's no source).
Note that this extra style is required because the itemTemplate feature of CKEditor autocomplete
is not powerful enough to support our specific usecase.
since 16.10.3
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
since 16.10.3
@since 16.10.3

We don't usually mention since version in CSS. It looks a bit strange without the @ but I'll leave it to you to decide (remove it, keep it as it or add @).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants