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

Sidebar and nav-tab-custom: additional CSS for focus state #1955

Merged
merged 4 commits into from
Nov 3, 2021
Merged

Sidebar and nav-tab-custom: additional CSS for focus state #1955

merged 4 commits into from
Nov 3, 2021

Conversation

rdwebdesign
Copy link
Member

Signed-off-by: rdwebdesign github@rdwebdesign.com.br

By submitting this pull request, I confirm the following:

  • I have read and understood the contributors guide, as well as this entire template.
  • I have made only one major change in my proposed changes.
  • I have commented my proposed changes within the code.
  • I have tested my proposed changes.
  • I am willing to help maintain this change if there are issues with it later.
  • I give this submission freely and claim no ownership.
  • It is compatible with the EUPL 1.2 license
  • I have squashed any insignificant commits. (git rebase)
  • I have Signed Off all commits. (git commit --signoff)

What does this PR aim to accomplish?:

Fixes #1843, improving accessibility for sidebar and nav-tabs.

How does this PR accomplish the above?:

Added CSS for :focus state, into every theme: default-light.css, default-dark.css, and default-darker.css.
Each theme has been modified accordingly.

What documentation changes (if any) are needed to support this PR?:

none

PromoFaux and others added 3 commits September 29, 2021 21:45
Signed-off-by: rdwebdesign <github@rdwebdesign.com.br>
@yubiuser yubiuser linked an issue Nov 1, 2021 that may be closed by this pull request
@yubiuser
Copy link
Member

yubiuser commented Nov 1, 2021

Thanks for fixing the issue.
However, I have two remarks:

  1. On the light setting, I see a blue line when focusing with the mouse, but a grey line with keyboard
  2. On the midnight theme I don't see a visual indication at all

I think I would prefer to have the line blue in all cases.

Bildschirmfoto zu 2021-11-01 13-02-49
Bildschirmfoto zu 2021-11-01 13-03-14

@rdwebdesign
Copy link
Member Author

@yubiuser

Wait.
focus is different than hover.

Light Theme (default-light.css):

Before the modifications we had a blue border on "hover", but there was no different visual on focus.
Now, we still have a blue border on "hover", but the text is white and I added a border on focus too.
The focus border color is different than the hover, but I can change that.
light

Midnight Theme (default-dark.css):

Before we had white text and darker background on "hover", but no visual effects on focus.
Now, hover is the same, but on focus the text is white and I added a border.
midnight_theme

Deep Midnight Theme (default-darker.css ):

Before we had white text and darker background on "hover", but no visual effects on focus.
Now, hover is the same, but on focus the text is white (the difference is small).
deep-midnight

Note: images showing the sidebar and nav-tabs using only the keyboard.

Any suggestions?

@yubiuser
Copy link
Member

yubiuser commented Nov 1, 2021

Thanks for the nice screenshots.
From my usability point of view hover and focus are even here: when I use the mouse, I hover and one click opens the resp. menu. If I use the keyboard, I focus and one "return" opens the menu. Have I overlooked something?

Therefore hover and focus can/should be the same I think - or rephrasing: I don't see a need to distinguish them. I like the border esp. on the left side menu and would add it to all of the themes.

P.S. Add it to the new theme as well ;-)

@rdwebdesign
Copy link
Member Author

Focus and Hover styles can be different or not. It's a design choice.
In this case, I agree that we don't need to distinguish between then. I can change that.

... I like the border esp. on the left side menu and would add it to all of the themes.

What do you mean?
The deep-midnight theme doesn't have it.
Actually, there is a border-color (on hover CSS), but there is no border for the element.
I thought that was somebody else's design choice, made before and I don't want to change it without more information.

P.S. Add it to the new theme as well

The new theme already has focus effects for the sidebar menu.

@rdwebdesign
Copy link
Member Author

rdwebdesign commented Nov 1, 2021

Just to add to our conversation...

From the usability point of view we have many states:

  • Normal state (nothing is happening);
  • Hover state (ex: mouse pointer is over a button or link);
  • Active state (ex: you are clicking a button or a link);
  • Focus state (ex: you used TAB key to move the focus)

The focus state will happen using the mouse too (interactive elements only: links, buttons, input elements, etc.):

  • if you selected an element with a click, the element gets the focus. Then, when you remove the mouse pointer after that, the focus will stay on the element until another action changes the focus again.

We don't normally need focus state, but some people with disabilities cannot use the mouse and need a visual cue to know where the focus is.

In the sidebar, using the mouse, you usually don't see the focus because you click and the focus disappears immediately when the new page loads, but using the keyboard you can navigate using TAB and select/fire the desired action using Return or ENTER.

@yubiuser
Copy link
Member

yubiuser commented Nov 2, 2021

The deep-midnight theme doesn't have it.
Actually, there is a border-color (on hover CSS), but there is no border for the element.
I thought that was somebody else's design choice, made before and I don't want to change it without more information.

I think it would benefit from one. But askIn the sidebar, using the mouse, you usually don't see the focus because you click and the focus disappears immediately when the new page loads.

I noticed this as well: it also happens if you click on the sidebar and keep the button pressed. It's most obvious on the light theme as (so far) it has different colors for hovering and focussing. ing @DL6ER who created the theme initially.


We don't normally need focus state, but some people with disabilities cannot use the mouse and need a visual cue to know where the focus is.

That's why I would add a "border for the element" to make the focus even clearer.

Signed-off-by: rdwebdesign <github@rdwebdesign.com.br>
@rdwebdesign
Copy link
Member Author

@yubiuser

I changed the CSS for the sidebar.
Now using the mouse or the keyboard you will see the same effect.

Each theme will change the text color, background-color and left-border (with the same color on both states) for :hover and :focus ;

@yubiuser yubiuser merged commit c0cd21f into pi-hole:devel Nov 3, 2021
@rdwebdesign rdwebdesign deleted the sidebar_focus branch November 3, 2021 22:55
@DL6ER DL6ER mentioned this pull request Dec 22, 2021
@pralor-bot
Copy link

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/pi-hole-ftl-v5-12-web-v5-9-and-core-v5-7-released/51795/1

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.

No visual indicators of side menu items keyboard focus
4 participants