-
-
Notifications
You must be signed in to change notification settings - Fork 552
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-21115: @breadcrumb-color doesn't apply to links in the Breadcrumb #3536
base: master
Are you sure you want to change the base?
Conversation
…own links * Updated the color used by the links in the breadcrumb dropdown.
* Updated the color used by the links in the breadcrumb bar.
color: @breadcrumb-color; | ||
&:hover, &:focus { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about the :active
and :visited
states?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically, this looks good. However, looking at this pull request and the issue again I wonder if there is an agreement that these colors should actually be applied to the links. I think it would be good to have small proposal to agree about this. The issue also mentions introducing new color variables for the links as an alternative from what I understand. |
Started https://forum.xwiki.org/t/breadcrumb-links-color/15735 👍 |
Setting it as a draft:
If option 1. is picked (the one currently implemented), make sure to correct the looks of this solution with the default Iceberg, |
Jira URL
https://jira.xwiki.org/browse/XWIKI-21115
Changes
Description
Screenshots & Video
Here, I set the color
@breadcrumb-color
to a lightgreen in order to be able to see it easily.Before the changes proposed in this PR:
After the changes proposed in this PR (also changed the
@breadcrumb-active-color
to a dark green to make sure it still works well together):Executed Tests
Manual tests, see above. This is a style change only, and it doesn't change the UI layout, so it should be pretty safe for automated tests.
Expected merging strategy