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

Modify css for better web accessibility #90

Merged
merged 12 commits into from
Aug 10, 2022

Conversation

Revathyvenugopal162
Copy link
Contributor

@Revathyvenugopal162 Revathyvenugopal162 commented Aug 5, 2022

This PR mainly focusing on:

  • Change the font size and color of main heading
  • Reduce the column 1 (column with search bar button)
  • Change the code block color back to pink
  • dropdown list button visibility. Fix Problems with dropdown list #89
    ansys-sphinx-theme-new1
    ansys-sphinx-theme-new2
    ansys-sphinx-theme-new3

@Revathyvenugopal162 Revathyvenugopal162 added the enhancement General improvements to existing features label Aug 5, 2022
@Revathyvenugopal162 Revathyvenugopal162 self-assigned this Aug 5, 2022
@jorgepiloto
Copy link
Member

Thanks for addressing this, @Revathyvenugopal162. Let me ping here @timdansys so he is aware about the changes we introduce regarding accessibility.

Tim is still working on his notes and will share those in the incoming days.

@jorgepiloto
Copy link
Member

From our private discussion, I think that inline code snippets (i.e. anything between ``) should also have a gray background. See this example in pydata-sphinx-theme.

@PipKat
Copy link
Member

PipKat commented Aug 5, 2022

@Revathyvenugopal162 Your changes look good to me. I'll let Jorge, Maxime, or Roberto be the approver though. The one thing I do remember Tim talking about is our possibly using gray boxes like this for inline code (literal tagging):

image.

However, I went to https://new.scipy.org/ to see what the doc for all the big scientific packages looked like. Sphinx was used for all but the IPthon doc, which was far less attractive and usable. All the the others used Sphinx, and of those, SymPy was the only one that used a custom-branded green color scheme that isn't appealing to me at all! Consequently, I'm OK with the pinkish-red color.

@PipKat
Copy link
Member

PipKat commented Aug 5, 2022

From our private discussion, I think that inline code snippets (i.e. anything between ``) should also have a gray background. See this example in pydata-sphinx-theme.

Jorge's suggestion is a great compromise. I think in the gray highlighted box, the pinkish-red text is easier to read than the black text.

@Revathyvenugopal162
Copy link
Contributor Author

Added the inline code gray background.Thanks @PipKat and @jorgepiloto for pointing this

Copy link
Member

@RobPasMue RobPasMue left a comment

Choose a reason for hiding this comment

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

LGTM apart from the mentioned comments and @jorgepiloto's and @PipKat's suggestions. Can you update the screenshots to the latest versions after your changes?

src/ansys_sphinx_theme/static/css/ansys_sphinx_theme.css Outdated Show resolved Hide resolved
src/ansys_sphinx_theme/static/css/ansys_sphinx_theme.css Outdated Show resolved Hide resolved
@RobPasMue
Copy link
Member

screenshot

By the way, I thought we were going to change the color and way of showing the top bar menus when hovering or accessing (e.g. using an underline). Will this be handled in this PR @Revathyvenugopal162?

@jorgepiloto
Copy link
Member

I think we can implement the "chapter underline" in a separate pull request, @RobPasMue @Revathyvenugopal162.

Also, @timdansys still needs to share with use his final notes about the new style. I would be nice to implement the proposed changes in issues to have a better idea on what has been implemented so far.

Comment on lines +472 to +480
###############
Dropdown button
###############
*/

.navbar button.navbar-toggler {
margin-right: 1em;
border-color: rgb(255, 255, 255);
color: rgb(255, 255, 255);
Copy link
Contributor

Choose a reason for hiding this comment

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

@Revathyvenugopal162 Thanks for adding it. 👍

Co-authored-by: Roberto Pastor Muela <roberto.pastormuela@ansys.com>
@MaxJPRey
Copy link
Contributor

MaxJPRey commented Aug 8, 2022

From our private discussion, I think that inline code snippets (i.e. anything between ``) should also have a gray background. See this example in pydata-sphinx-theme.

@jorgepiloto and @PipKat I agree, the contrast is better than for numpy

For example:
image

@Revathyvenugopal162
Copy link
Contributor Author

code block
Adding the code inline boxes image.

@da1910
Copy link

da1910 commented Aug 8, 2022

I'm not entirely sure if it's worth making a new issue for this, but if we're thinking about accessibility - the contrast for the blue links, pink code/quote sections, and orange syntax highlighting in light mode is below the recommended value:

image

image

image

The WCAG guidelines - https://www.w3.org/WAI/standards-guidelines/wcag/ are probably worth following in this case, and aiming for AA level is quite reasonable (not much change needs to be made), aiming for AAA would be a much more obvious change.

Below is the suggested minimum contrast level in light mode:
image

Links - #0077B3
Quotes - #CD186D
Syntax - #B35000

@Revathyvenugopal162
Copy link
Contributor Author

Thanks @da1910 for the color values. Adding these values.

@@ -116,7 +116,7 @@ html[data-theme="dark"] {
* layout
*/

--pst-color-link: #0088CC;
--pst-color-link: #0077B3;
Copy link

Choose a reason for hiding this comment

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

We shouldn't change this, #0077B3 also fails accessibility guidelines in dark mode.

Copy link

@da1910 da1910 left a comment

Choose a reason for hiding this comment

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

Looks good for contrast now!

@Revathyvenugopal162
Copy link
Contributor Author

Merging. Thanks @MaxJPRey @jorgepiloto @RobPasMue @PipKat @da1910 for the thorough review.

@Revathyvenugopal162 Revathyvenugopal162 merged commit 53d7417 into main Aug 10, 2022
@Revathyvenugopal162 Revathyvenugopal162 deleted the feat/modify-css-for-color-enhancement branch August 10, 2022 13:32
@jorgepiloto jorgepiloto mentioned this pull request Aug 11, 2022
22 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement General improvements to existing features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Problems with dropdown list
6 participants