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

Address accessibility for syntax highlighting #9273

Merged
merged 2 commits into from
Oct 10, 2022
Merged

Conversation

lyndaidaii
Copy link
Contributor

@lyndaidaii lyndaidaii commented Oct 5, 2022

Approach: use our own syntax highlighting css style to have more control over

contrast issue:

  • Change color from #d73a49 to #cc3745
  • Change from #e36209 to #b74e05
    Test: Ran few packages with code fence readme with FastPass

Addresses #9269

@lyndaidaii lyndaidaii requested a review from a team as a code owner October 5, 2022 20:10
@joelverhagen
Copy link
Member

This looks like a "surgical" approach. I presume the goal is to minimize the apparent change in how it looks, as opposed to using one of the accessible CSS stylesheets?

I see a lot more hex colors mentioned than the two in your PR description. What is the intended scope of changes?

What methodology did you use to select the new colors?

Could you provide a representative screenshot showing the difference?

Copy link
Member

@joelverhagen joelverhagen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some questions about approach. Overall, looks reasonable though.

@lyndaidaii
Copy link
Contributor Author

lyndaidaii commented Oct 6, 2022

This looks like a "surgical" approach. I presume the goal is to minimize the apparent change in how it looks, as opposed to using one of the accessible CSS stylesheets?

I see a lot more hex colors mentioned than the two in your PR description. What is the intended scope of changes?

What methodology did you use to select the new colors?

Could you provide a representative screenshot showing the difference?

I think we want to have our own style so that we could have more customization about how it looks like.
The reason why there is color contrast of original highlight github min style we were using is that we changed background to grey(#f6f8fa) instead of white in the first PR to match with devops code fence style.
The fast pass shows suggest color to avoid color contrast. that how I picked up new color to avoid contrast. Color changes are small, hard to tell the difference with original color and new color.
Before change:
beforeAcc
After change:
afterAcc

@joelverhagen
Copy link
Member

The "after" looks great still. Thanks for the additional information, and nice work on the rapid fix. Consider testing a sample on each of the top 10-20 syntax highlighting languages. It's not clear to me that different languages use colors consistently so testing a broad set of languages might be prudent (I'm thinking of you, F# 😄)

@lyndaidaii
Copy link
Contributor Author

The "after" looks great still. Thanks for the additional information, and nice work on the rapid fix. Consider testing a sample on each of the top 10-20 syntax highlighting languages. It's not clear to me that different languages use colors consistently so testing a broad set of languages might be prudent (I'm thinking of you, F# 😄)

I tested all packages that have accessibility issues with FastPass. Thanks for reminder about other languages test!

@lyndaidaii lyndaidaii merged commit 0e42bd0 into dev Oct 10, 2022
RiadGahlouz added a commit that referenced this pull request Oct 19, 2022
* Update NuGetGallery.Services to address CG alerts (#9274)

* Address accessibility for syntax highlighting (#9273)

* address accessbility for syntax highlighting

* Fix Support link (#9276)

* fix broken support link

* Update Download table heading (#9267)

* update correct place

* update table header placement

* Add instructions to install MSBuildSdk packages (#9268)

* Added "IsMSBuildSdkPackageType" to determine whether a package is of type MSBuildSdk.

DisplayPackage view modified to show specific instructions for SDK types in project files as per #8800

* Changed "Include" to correct attribute "Name" for SDK package type

Co-authored-by: Advay Tandon <82980589+advay26@users.noreply.github.com>
Co-authored-by: lyndaidaii <64443925+lyndaidaii@users.noreply.github.com>

* [CodeQL] Suppress CSRF token validation warnings (#9278)

* Added CSRF token checks to address CodeQL bugs

* Added CodeQL suppressions

* Make thinner border for focused links (#9277)

* Make thiner border for focused links

* Change border size of package manager tabs

* Delete comment line from base.less

* Change nav-tabs color and make overflow-y visible for package-tags

Co-authored-by: Joel Verhagen <jver@microsoft.com>
Co-authored-by: lyndaidaii <64443925+lyndaidaii@users.noreply.github.com>
Co-authored-by: toseni <seniut.tomas@gmail.com>
Co-authored-by: Ian Rathbone <ian@rathbone.dev>
Co-authored-by: Advay Tandon <82980589+advay26@users.noreply.github.com>
Co-authored-by: Daniel Olczyk <44818681+MRmlik12@users.noreply.github.com>
@joelverhagen joelverhagen deleted the AccFixForHighlighting branch August 22, 2024 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants