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

consider underlines for links #203989

Closed
meganrogge opened this issue Feb 1, 2024 · 15 comments · Fixed by #205205
Closed

consider underlines for links #203989

meganrogge opened this issue Feb 1, 2024 · 15 comments · Fixed by #205205
Assignees
Labels
accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues feature-request Request for new features or functionality on-testplan themes Color theme issues verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@meganrogge
Copy link
Contributor

meganrogge commented Feb 1, 2024

We currently distinguish links using only color, no underline in some places.

This can make it hard for color blind or low vision users to differentiate between links and regular text.

https://aka.ms/MAS1.4.1

Underlines can also reduce readability though.

Thoughts @hbons or @isidorn ?

https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1881734

Screenshot 2024-02-01 at 11 07 51 AM
Screenshot 2024-02-01 at 11 07 59 AM

@meganrogge meganrogge self-assigned this Feb 1, 2024
@meganrogge meganrogge added accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues under-discussion Issue is under discussion for relevance, priority, approach labels Feb 1, 2024
@setaskin
Copy link

setaskin commented Feb 1, 2024

Hi all, Live Share has a sev2 bug assigned for it. It effects our Accessibility Grade. I filed an extension request for it.

@hbons
Copy link
Member

hbons commented Feb 1, 2024

How about we start with enabling this in the High Contrast themes?

@hbons hbons self-assigned this Feb 1, 2024
@meganrogge meganrogge added bug Issue identified by VS Code Team member as probable bug feature-request Request for new features or functionality and removed bug Issue identified by VS Code Team member as probable bug under-discussion Issue is under discussion for relevance, priority, approach labels Feb 1, 2024
@hbons hbons added the themes Color theme issues label Feb 1, 2024
@hbons
Copy link
Member

hbons commented Feb 5, 2024

This one is trickier than I thought. VS Code uses <a> elements throughout the IDE also for things like tabs and buttons, so we need to find a way to only match the elements that look like text...

edit: we can match [role=link].

@meganrogge
Copy link
Contributor Author

I noticed that we do this in the editor even for non-HC themes.
Screenshot 2024-02-05 at 12 32 10 PM

@hbons
Copy link
Member

hbons commented Feb 6, 2024

After some investigation, matching [role=link] doesn't work as it's the default role and we can only match it if it's explicitly set in the a element. Looking for some other solutions, but we may need to add the role attribute explicitly everywhere.

@hbons
Copy link
Member

hbons commented Feb 13, 2024

@isidorn do you have advice how we might work around this?

@isidorn
Copy link
Contributor

isidorn commented Feb 13, 2024

We do not have an issue filled for this as is my understanding.
LiveShare has it, and filling extension is the right approach (which was already done).

So for now I suggest no action here.

@hbons
Copy link
Member

hbons commented Feb 14, 2024

I still think this is a good thing to look into. Our links in the HC themes don't look very contrasty in any case:

image

I realised we don't need to underline all links, just those that appear in a text. So matching p a should do the trick.

@isidorn
Copy link
Contributor

isidorn commented Feb 14, 2024

@hbons that's a fair point. So could we fix for High Contast only first?

@hbons
Copy link
Member

hbons commented Feb 14, 2024

@isidorn yes, we'll keep it to HC themes for now. I've managed to get it to match only relevant text links that have surrounding text. As you can see standalone text links won't be underlined. GitHub also does not do this.

I think it looks pretty good (ignore the red colour, it's to check we're matching the right things and I will reset it):

image image image

@hbons
Copy link
Member

hbons commented Feb 14, 2024

Specially in the light HC theme it's a big improvement:
image

@isidorn
Copy link
Contributor

isidorn commented Feb 14, 2024

Looking good for the HC theme. Thank you

@VSCodeTriageBot VSCodeTriageBot added the unreleased Patch has not yet been released in VS Code Insiders label Feb 14, 2024
@VSCodeTriageBot VSCodeTriageBot added this to the February 2024 milestone Feb 14, 2024
@VSCodeTriageBot VSCodeTriageBot added insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Feb 15, 2024
@rzhao271 rzhao271 added the verification-needed Verification of issue is requested label Feb 19, 2024
@alexr00
Copy link
Member

alexr00 commented Feb 20, 2024

Should these have underlines in HC?
image

Otherwise, verified in welcome views and chat.

@alexr00 alexr00 added the verified Verification succeeded label Feb 20, 2024
@hbons
Copy link
Member

hbons commented Feb 20, 2024

@alexr00 these don't need to be underlined. Only links in a paragraph surrounded by other text.

@daviddossett daviddossett reopened this Jun 12, 2024
@VSCodeTriageBot VSCodeTriageBot removed the insiders-released Patch has been released in VS Code Insiders label Jun 12, 2024
@daviddossett
Copy link
Contributor

Adding setting to enable link underlines in #216842

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues feature-request Request for new features or functionality on-testplan themes Color theme issues verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants