XWIKI-20827: Inline links must be distinguishable without relying on color #2329
Conversation
…color * Darkened color of the regular text on a themeless wiki and Iceberg colortheme. * Added a rule to underline anchors in `muted-text` nodes. Note: * It was not possible to just make the muted text color darker, because it would have been too similar to the regular text color. * From my manual tests, this solved the `link-in-text-block` in all situations. Tested on the Home, Sandbox and Help pages.
…color * Increased precision on the text muted selector to avoid filling the breadcrumb with nodes that don't need an underline but had one.
|
My two cents but I think this is really a design and not a code review questions:
Overall, to me this seems better than the other proposed solutions. Maybe it would be a good opportunity to question the use of muted-text. I'm not saying we should remove it, I'm just saying that we should check where it is used and if it really makes sense there. I would suggest updating the forum thread with this solution to get some more feedback. |
|
Thank you for your feedback @michitux !
For muted-text, there is no 'easy' solution by just changing color. Therefore we need to make a selector (just like the solution we had to abandon in #2313) to detect inline links. However, here the selector is easier, since we can assume that everything that uses the Let me know if you think of a better solution, but even though we discussed a lot on the topic I couldn't think of a better one yet.
Note that I took care of some edge cases in e19d056. Before that, all anchors in the breadcrumbs would be underlined even though they are not inline. This commit limits the use of underline to the most straightforward kind of use of the mixin: muted textual element with an anchor node inside. I think it would be difficult to find anywhere where the use of .muted-text is not appropriate:
I reopened the topic and posted: |
|
Note that Github seems to be heading towards a new option to underline links: |
Yes, and they say what we said, i.e. not everyone wants underline so it needs to be an option. |
| // contrast between anchors and regular text is too low. | ||
| .text-muted { | ||
| color: @text-muted; | ||
| & > a { |
There was a problem hiding this comment.
Isn't this selector too restrictive? For example, from wiki syntax, we frequently wrap links in a span.
There was a problem hiding this comment.
Note that I took care of some edge cases in e19d056. Before that, all anchors in the breadcrumbs would be underlined even though they are not inline. This commit limits the use of underline to the most straightforward kind of use of the mixin: muted textual element with an anchor node inside.
It's not a perfect selector unfortunately... :/
From my understanding, if we wrap the link in a span, then it's correct to assume the '.text-muted' is only applied on this span. From what I could see, this selector was appropriate and allowed to avoid getting too many underlines everywhere (the breadcrumb ones were not that good).
From what I could see it worked well on the few pages I tested on (Home, Help, Sandbox and change history).
…color * Fixed an issue with the underlining disappearing on the Help page
* Reverted css changes related to muted-text
* Fixed unwanted change
|
Removed the changes related to the |
Jira
https://jira.xwiki.org/browse/XWIKI-20827
PR Changes
muted-textnodes.Notes
link-in-text-blockin all situations. Tested on the Home, Sandbox and Help pages.View
First, here is a comparison of the Home page with the Iceberg color theme, with and without the changes in this PR:
vvv before the PR
vvv after the PR
vvv Side by side view
Hereafter are views of other pages in a default distribution, with the changes in this PR. Note that some anchors are underlined when used in blocks with the .muted-text mixin.
At last, a full color comparison between
#333(former text-color value) and#222(new text-color value):