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-20827: Inline links must be distinguishable without relying on color #2313

Closed
wants to merge 1 commit into from

Conversation

Sereza7
Copy link
Contributor

@Sereza7 Sereza7 commented Aug 29, 2023

Jira

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

PR Changes

  • Added the 'text-decoration: underline' rule to inline links, using a node nature selector.

View

Here is the home page of XWiki standard after the changes in this PR. We can see that inline links are now underlined. This solves the WCAG issues related to indistinguishability of inline links reported in this issue:
20827-afterPR

Here is the same screenshot, improved with highlights around links (orange: not underlined by the PR; cyan: underlined by the PR):
20827-afterPRHighlight

For reference, here is a similar screen I took before this PR, highlighting in the same way what links should get underlined:
InlineAndOutlineLinksExample

Tests

Manual tests have been conducted on a few pages of XWiki Standard (e.g. the Home page seen above), this change seems to behave as expected.
This is only a style change, so I didn't run docker tests.

Note

This is not a perfect fix, but it should cut down the amount of violations of link-in-text-block in the CI (so far, 373 out of the total 585 violations). For example:

  • A list, with a wikilink inline, will not underline it.
  • A paragraph with just a link (not inline), will be underlined.

This PR makes assumptions on the content of the parents of the links, and those assumptions are correct in most cases, not all cases.

…color

* Added the 'text-decoration: underline' rule to inline links, using a nature based selector.

Notes: Manual tests have been conducted on a few pages of XWiki Standard, this change seems to behave as expected.
@manuelleduc
Copy link
Contributor

Did you consider the case of buttons that looks like links, for instance <button type="button" class="btn btn-link" disabled>Link</button> ?

@Sereza7
Copy link
Contributor Author

Sereza7 commented Aug 30, 2023

@manuelleduc

Did you consider the case of buttons that looks like links, for instance <button type="button" class="btn btn-link" disabled>Link</button> ?

I did not consider this case, however, since those do not have a .wikilink, .wikiexternallink, .wikiinternallink, .wikicreatelink, they are not affected by the changes. I think this makes sense, since they are not inline, and are already contained in their own little block (even if this block doesn't have a background). On the screenshot below, I think it's easy to separate the link from neighboring text, even without the difference in color. So in my opinion those links do not need to be underlined and the behavior of this PR is correct. Thank you for the help! This would have been problematic if the behavior didn't match the expectations.
20827-buttonLink

@michitux
Copy link
Contributor

michitux commented Sep 5, 2023

I'm convinced by this selector. While I agree that it should work in most cases, the fact that it doesn't work with lists is really not nice in my opinion. The problem for me is that this creates an inconsistency which is not nice. Imagine some paragraphs of text with some links and then a list that also contains some links. This will look quite inconsistent in my opinion. I unfortunately don't have any better solution, though. Maybe, if we cannot target these links reliably, making the text darker is the better solution in the end?

@Sereza7
Copy link
Contributor Author

Sereza7 commented Sep 5, 2023

Making the anchor text darker will imply also making regular text darker on Iceberg at least. And we have to also consider the :visited color too.

We could also process the link style with javascript--jquery, but I wanted to avoid managing styles with javascript.

Do you think I should close this PR and start another PR about an alternative involving text colors, or continue on this idea and make a more precise 'selector' for inline links with JQuery?

Thank you for your feedback :D

@michitux
Copy link
Contributor

michitux commented Sep 5, 2023

I meant making the regular text darker, not making the anchor text darker. The problem with the JavaScript solution is WYSIWYG editing. You'll either use the WYSIWYG-property or you'll risk saving extra attributes on links unless you can manage to set the anchor style without modifying the resulting HTML.

@Sereza7
Copy link
Contributor Author

Sereza7 commented Sep 7, 2023

Closing this PR since there is now another (hopefully more appropriate) solution to fix this issue at #2329.

@Sereza7 Sereza7 closed this Sep 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants