Add a link to skip to the main content #5396
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
javierm
force-pushed
the
fix_new_window_typo
branch
from
cb6e23c
to
7d53a1a
Compare
javierm
force-pushed
the
fix_new_window_typo
branch
from
7d53a1a
to
25bf231
Compare
javierm
force-pushed
the
fix_new_window_typo
branch
from
25bf231
to
8ace0e1
Compare
javierm
force-pushed
the
main_section
branch
2 times, most recently
from
c75d973
to
e57660e
Compare
javierm
force-pushed
the
main_section
branch
3 times, most recently
from
5112c0d
to
b86f7e3
Compare
javierm
force-pushed
the
fix_new_window_typo
branch
from
8ace0e1
to
8ca91a7
Compare
javierm
force-pushed
the
fix_new_window_typo
branch
from
8ca91a7
to
1a22db8
Compare
javierm
force-pushed
the
main_section
branch
6 times, most recently
from
24b3e27
to
6e43256
Compare
taitus
approved these changes
Mar 21, 2024
There can only be one <main> tag in a document, and we've already got a <main> tag in the management layout.
We were testing what happens when clicking on a geozone without HTML coordinates, which won't happen in a real browser. So we're now defining the HTML coordinates and clicking on the area in the test, which is what real people will do. We also avoid having two consecutive `visit` calls, which will interfere with the way we plan to test the presence of the <main> tag after every `visit`. Note that, the test didn't work with the HTML coordinates defined in the `with_html_coordinates` trait, with Capybara showing the following error: ``` Selenium::WebDriver::Error::ElementClickInterceptedError: element click intercepted: Element <area shape="poly" coords="30,139,45,153,77,148,107,165" href="/proposals?search=California" title="California" alt="California"> is not clickable at point (413, 456). Other element would receive the click: <img usemap="#map" src="/assets/map.jpg"> ``` The cause of this error was the strange shape of the polygon, which was greatly concave and and so the middle of its area wasn't part of it. We're changing the polygon so it's now convex and when Capybara clicks on its middle point everything will work as expected.
Many pages had this tag, but many other didn't, which made navigation inconsistent for people using screen readers. Note that there are slight changes in two pages: * The homepage now includes the banner and the content of the `shared/header` element inside the <main> tag * The budgets index now includes the banner inside the <main> tag I see both potential advantages and disadvantages of this approach, since banners aren't necessarily related to the main content of a page but on the other hand they aren't the same across pages and people using screen readers might accidentally skip them if they jump to the <main> tag. So I'm choosing the option that is easier to implement. Note we're adding a `public-content` class to the <main> element in the application layout. This might be redundat because the element could already be accessed through the `.public main` selector, but this is consistent with the `admin-content` class used in the admin section, and without it the <main> element would sometimes have an empty class attribute and we'd have to use `if content_for?(:main_class)` or `tag.main` which IMHO makes the code less consistent. The Capybara::DSL monkey-patch is only done on the `visit` method because it's the only reliable one. Other methods like `click_link` generate AJAX requests, so `expect(page).to have_css "main", count: 1` might be executed before the AJAX request is finished, meaning it wouldn't properly test anything.
While people using screen readers already have keyboard shortcuts to jump to the <main> tag, there are people who navigate the page with the keyboard using just the tab key, and for them, this link provides a way to save time and start reading the main content instead of having to manually go through all the navigation links every time a new page is loaded. Note that we had to add an additional `width: 0` rule because Foundation's `element-invisible` would apply `1px` and the test checking for `visible: :hidden` would faile.
This way our views are more consistent.
This is consistent with the way we're providing the main class. Note we're still setting the title using a block in more complex cases.
|
Big thanks to @decabeza, who made me realize that not all keyboard users have keyboard shortcuts to navigate to the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
References
Objectives
<main>element in every pageVisual changes
After these changes