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.

Sign up for GitHub

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

Tabbing highlights #177

Closed
wants to merge 8 commits into from
Closed

Tabbing highlights #177

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e22b20c
fix acessibility hover
jbbain Jul 7, 2022
65c8f6a
fix accessibility tabbing for home page
jbbain Jul 7, 2022
a81e719
Merge branch 'main' into tabbing-highlights
jaredbain Jul 14, 2022
ff315f2
Merge branch 'main' into tabbing-highlights
jaredbain Aug 23, 2022
011b8cc
Merge branch 'main' into tabbing-highlights
jaredbain Sep 15, 2022
8d124d6
Merge branch 'main' into tabbing-highlights
DaveSkender Dec 17, 2022
beb76fb
Merge branch 'main' into tabbing-highlights
jaredbain Sep 6, 2023
a645d7d
add outline-offset
jbbain Sep 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions _assets/css/component/_boxes.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,14 @@
outline-color: color("primary-vivid");
outline-width: 8px;
}
&:focus-visible {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not exactly sure what's causing it, but there seems to be an extra tab action (two per box) that is weird.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i'll look into this

@include u-bg("transparent");
outline-style: solid;
outline-color: color("primary-vivid");
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the right color? To be consistent with the others?

Copy link
Contributor

@DaveSkender DaveSkender Jul 10, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

may also want to try outline-offset: units(0.5); to be consistent with other buttons

Copy link
Contributor

Choose a reason for hiding this comment

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

there's probably some variable settings you can use here:

$theme-focus-color: "blue-40v" !default;
$theme-focus-offset: 0 !default;
$theme-focus-style: solid !default;
$theme-focus-width: 0.5 !default;

Ref: default settings

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah this is the right color, I copied the exact styling that is already there for the original hover for the other buttons

Copy link
Contributor

Choose a reason for hiding this comment

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

Hover and focus may be different colors, not sure. I suspect our custom "primary-vivid" override is actually blue-50v, instead of the default blue-40v for focus; it's hard for me to ascertain.

image

outline-width: 8px;
outline-offset: units(0.5);

}

a {
@include u-margin(0);
Expand All @@ -129,6 +137,10 @@
@include u-bg("transparent");
text-decoration: none;
}
&:focus-visible {
@include u-bg("transparent");
text-decoration: none;
}
}

.usa-button {
Expand Down
5 changes: 5 additions & 0 deletions _assets/css/pages/_fellows.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,11 @@
text-decoration: underline;
}
}
&:focus {
h2 {
text-decoration: underline;
}
}
.body-intro--bold {
color: color('primary');
@include u-font('serif', 9); // 22 px -- h2 sizing overrides the typography.scss size declaration
Expand Down
4 changes: 2 additions & 2 deletions pages/home.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ <h3 class="twoline">Mentorship and <br class="display-none tablet:display-inline

<!-- impact -->
<div class="grid-row">
<div class="grid-col box-pane-button home-opportunity">
<div class="grid-col box-pane-button home-opportunity" tabindex="0">
Copy link
Contributor

Choose a reason for hiding this comment

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

seems odd to need to force tabindex here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for divs you need to add tabindex in order for focus to work because by default, it is for elements such as , < button>, or anything that is usually interactive with the user.

Copy link
Contributor

Choose a reason for hiding this comment

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

yah, I think I remember struggling with this as it's not a standard HTML element. we do have the a element under it, but I don't think that's what receives the focus here.

<a class="usa-button" href="{{site.baseurl}}/opportunity/">

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can use tab index=“-1” on the a to skip the tab stop, but should add an aria-labelledby on the parent div, with reference back to the a child element, to keep a proper accessibility schema. Something to try.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So this does fix the double tabbing however the div isn't redirecting to the other page because it is not the link element.

<a class="usa-button" href="{{site.baseurl}}/opportunity/">
<div class="box-pane">
<h2>The Opportunity</h2>
Expand All @@ -124,7 +124,7 @@ <h2>The Opportunity</h2>

<!-- mission and values -->
<div class="grid-row">
<div class="grid-col box-pane-button home-about">
<div class="grid-col box-pane-button home-about" tabindex="0">
<a class="usa-button" href="{{site.baseurl}}/about/">
<div class="box-pane">
<h2>Who we are</h2>
Expand Down
Loading