Adds "verified domain" identifier to url tooltip by amaust · Pull Request #197037 · microsoft/vscode · GitHub
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

Adds "verified domain" identifier to url tooltip #197037

Merged
merged 6 commits into from
Nov 2, 2023

Conversation

amaust
Copy link
Contributor

@amaust amaust commented Oct 30, 2023

closes #196876

Adds "Verified Domain" to the hover tooltip of Verified domain extension's marketplace pages.

@amaust
Copy link
Contributor Author

amaust commented Oct 30, 2023

@meganrogge Downloading the extension from the file doesn't bring over the "Verified" status. So I will need you to test this for me.

I think this should fix it. However I am not extremely familiar with the codebase to know for sure if I'm in the right spot.

@meganrogge
Copy link
Contributor

thanks for working on this. In oss, I'm seeing it work when I shift+tab to the element, but not when I tab to it 🤔 . Pretty strange.

verified.mov

@amaust
Copy link
Contributor Author

amaust commented Oct 31, 2023

Interesting. Do you have any suggestions? I'm not sure if my fix changed the Aria label?

@meganrogge
Copy link
Contributor

I'll need to investigate more tomorrow. I'm not sure why it would work when focusing it in the backward tab order and not the forward. I haven't seen anything like that before.

@amaust
Copy link
Contributor Author

amaust commented Oct 31, 2023

Sounds good.

@meganrogge
Copy link
Contributor

@sandy081 do you know why this would only work in one direction? could you pls test and see if you experience the same?

@sandy081
Copy link
Member

sandy081 commented Nov 2, 2023

This is because when focusing using tab first element to be focused is the container (name + verified publisher) whereas in the other direction verified publisher is being focused first.

image

@meganrogge
Copy link
Contributor

@amaust given that, do you want to add this to the container's aria label too?

@amaust
Copy link
Contributor Author

amaust commented Nov 2, 2023

Yep, sounds good

@amaust
Copy link
Contributor Author

amaust commented Nov 2, 2023

Just for clarification @meganrogge, the container would be outside of the widget itself correct? If so do you know where I should look to find the container?

@meganrogge
Copy link
Contributor

@amaust
Copy link
Contributor Author

amaust commented Nov 2, 2023

Should be good to go now @meganrogge.

@meganrogge
Copy link
Contributor

meganrogge commented Nov 2, 2023

On second thought, the verified domain will be indicated when the link is focused. I think that's what we want 👍🏼. We don't need to apply this to the container.

@meganrogge meganrogge added this to the November 2023 milestone Nov 2, 2023
@meganrogge meganrogge merged commit 2e0f6ef into microsoft:main Nov 2, 2023
6 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Dec 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Accessibility] Verified domain in Extension page doesn't have a clear title attribute
4 participants