fix(link): update visited link color #8177
Conversation
|
Deploy preview for carbon-elements ready! Built with commit 78f3f21 |
|
Deploy preview for carbon-components-react ready! Built with commit 78f3f21 https://deploy-preview-8177--carbon-components-react.netlify.app |
|
If we are setting visited styles all the time, do we have a need for the |
|
that will require confirmation from design, given the original PR which added the that being said, I believe changing to visited styles being opt-out would likely make the prop unneeded |
|
Yes we do want optional visited styles still. Not all products want the visited style showing up because it can look messy. |
emyarod
force-pushed
the
update-visited-link-styles
branch
from
2ce9b34
to
b0ca714
Compare
emyarod
force-pushed
the
update-visited-link-styles
branch
from
b46498b
to
78f3f21
Compare
| @@ -47,7 +47,7 @@ | |||
| } | |||
|
|
|||
| &:visited { | |||
| color: $link-01; | |||
| color: $visited-link; | |||
There was a problem hiding this comment.
I may be confused with this PR, but this change seems to imply we are no longer allowing teams to opt-out of having visited styles? This seems like it could cause unintended style issues in products with links that are navigated multiple times.
I believe the original intent was to provide visited styles but let teams allow them based on the visited prop, not enable them by default?
There was a problem hiding this comment.
this change seems to imply we are no longer allowing teams to opt-out of having visited styles
this PR allows teams to opt out of visited styles. can you explain where the implication is coming from?
There was a problem hiding this comment.
originally, visited styles were overridden which meant that teams did not have visited styles by default (referenced issues above) so visited styles were opt-in. but support for opt in visited styles was also just partial support due to the added requirement for a CSS class on top of the :visited pseudoclass
There was a problem hiding this comment.
this PR allows teams to opt out of visited styles. can you explain where the implication is coming from?
This PR is setting all visited links to purple. When I go to the Link story and enter an address I've been to, I am seeing visited styles. How would a team, that does not want visited styles, opt-out from receiving visited styles?
We set the :visited state to the link color so that teams would not get visited styles by default, but could opt into them by passing a visited prop. I'm just looking for clarification that we are changing our stance and now want all teams to have visited styles by default, as this will have a large impact across product teams. Is there a linked issue that says that we want to enable this functionality by default?
There was a problem hiding this comment.
to opt out of visited styles teams can target the :visited pseudoclass as it was intended. I think opt-in visited styles were flawed to begin with because there is no way to know whether or not a user has visited a link due to privacy issues. for further clarification there is also the attached design spec at the top of the thread
I'm also not sure there's any large impact to returning to following the HTML spec. what kind of impact are you anticipating?
There was a problem hiding this comment.
My main concern is just that when product teams pull in this update, all visited links will be purple until they set their own :visited styles
There was a problem hiding this comment.
to me that change doesn't seem different from routine color contrast fixes or token updates, and the behavior would also fall in line with established usability patterns. I see the case for removing visited styles for use cases like the Gatsby theme or in brand/marketing but for the core Carbon component I think it would be a usability antipattern
There was a problem hiding this comment.
to me that change doesn't seem different from routine color contrast fixes or token updates
Not really, tokens aren't opt-in / out. There is just one value. If Cloud has guidance that visited links are the same color as normal links and Cloud PAL pulled in this update, they would then need to ensure they set the correct visited color. Based on Laurens's response in slack, this PR doesn't seem like it's needed.
We would also need to update guidance on the website if we change the default behavior
Happy to create a new issue that looks into the UX of this component and so we can get feedback from product teams about link behavior, but none of these linked issues are advocating that we should set visited styles on components by default.
There was a problem hiding this comment.
to clarify this PR was opened based on contradictory information which I received from design last week together with the spec image. although they have retracted that feedback and are now aligned I think the usability issues have been brought to light again (which is separate from whether or not downstream users need to update based on Carbon changes since that would be an expected consequence from reverting antipatterns)
that being said, the guidance would still remain the same, the Gatsby theme would remain unaffected, and the difference lies in what's exported in a themed package vs what's exported in the core component library. even though this issue has been brought up previously it isn't necessarily a hill I'm willing to die on but the lack of usability input on this subject is surprising esp since it directly led to an initial misdiagnosis of #8158
| visited: deprecate( | ||
| PropTypes.bool, | ||
| `\nThe prop \`visited\` for Link has been deprecated and will | ||
| be removed in the next major release.` | ||
| ), |
There was a problem hiding this comment.
If we want users to opt in to visited link styles (meaning by default, there are no visited link style differences), then maybe we shouldn't deprecate this prop. I can see it being used to conditionally add a className that will then add purple visited link styles. i.e. Thoughts? @tw15egan @emyarod @laurenmrice
There was a problem hiding this comment.
just FYI the basis of the PR is to return to opt-out rather than opt-in styles based on the confusion it caused even our own issue triaging, as well as established research e.g. 1 2 3
that being said, given that @dakahn has removed the request for review it seems this is not an a11y concern and primarily a ux one
There was a problem hiding this comment.
Yeah sorry, meant to comment. As long as contrast requirements are met and we have a logical consistency across the system with various states for links we're good from an accessibility and usability standpoint.
Related #3581
Related #5239
Related #7115
Related #8158
based on the latest spec from design
This PR restores visited link styles to the Link component. The removal of visited link styles was meant to be scoped to product and possibly dotcom, but it should remain in the core component library. The Gatsby theme should also be unaffected by this carbon-design-system/gatsby-theme-carbon#697
Testing / Reviewing
Confirm that visited Carbon links receive the correct styles by setting the Link story's
hrefknob to an address in your browser history