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.
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.
This link used to open in a new window, and we accidentally changed that
behavior while refactoring it in commit c2710de5f.
Since we're adding a test for this case, and the Proposals::NewComponent
class is similar, we're adding a test for that class too. In the case of
proposals, we need to sign in a user because the proposals form contains
fields to attach image, that currently rely on a user being signed in.
The `use_helpers` method was added in ViewComponent 3.8.0, and it's
included by default in all components since version 3.11.0.
Note we sometimes delegated the `can?` method to the controller instead
of the helpers, for no particularly reason. We're unifying that code as
well.
Before this change, two important things depend on the format of each key,
where to render it in the administration panel and which kind of interface
to use for each setting. Following this strategy led us to a very complex
code, very difficult to maintain or modify. So, we do not want to depend
on the setting key structure anymore to decide how or where to render each
setting.
With this commit, we get rid of the key format-based rules. Now we render
each setting explicitly passing to it the type and the tab where it belongs.
Instead of using a setting nested param `setting[:tab]`. We only need
the tab param when rendering settings in the administration section.
This change will make it easier rendering the correct tab after
updating settings.
This will help search engines know these links point to external sites
and it'll make it possible to style these links using the
`[rel~=external]` selector. AFAIK, assistive techonologies don't use
this attribute to notify people about external links, though.
This is a funny one, because we were accidentally opening them in the
same window without intending to do so since commit 928312e21, since the
`sanitize` method removes the `target` attribute. So the test we're
adding already passed without these changes.
Just like we're doing with other external links.
We already mention that it's an external video, so there's no need to
explicitly indicate it in the link.
As mentioned in earlier commits, opening external links in a new
tab/window results in usability and accessibility issues.
Since these links are usually at the top or bottom of the page and
contain icons of well-known sites, IMHO there's no need to even notify
people that these are external links.
Since we're no longer using the `shared.target_blank` translation inside
a sentence, we can remove the space and parenthesis in the translations.
In the admin section, when clicking on a link that leads to a page in
the public area, sometimes the page was opened in the same window and
sometimes it would open in a new window, with no clear criteria
regarding when either scenario would take place.
This was really confusing, so now we're more consistent and open
(almost) every link in the same window. The main reason behind it is
simple: if we add `target: _blank`, people who want to open those links
in the same window can no longer do so, so we're taking control away
from them. However, if we don't add this attribute, people can choose
whether to open the link on the same tab or to open it on a new one,
since all browsers implement a method to do so.
More reasons behind this decision can be found in "Opening Links in New
Browser Windows and Tabs" [1].
We're keeping some exceptions, though:
* Opening the link to edit an investment on the same tab would result in
losing all the investment filters already applied when searching for
investments, so until we implement a way to keep these filters, we're
also opening the link to edit an investment in a new tab
* For now, we're also opening links to download files in a new window;
we'll deal with this case in the future
[1] https://www.nngroup.com/articles/new-browser-windows-and-tabs/
Most screen readers don't notify when a link is about to open in a new
window [1], so we're indicating it, like we were already doing in most
places with similar links.
We could also add a visual indicator, but since links inside labels
already have accessibility issues, giving more attention to these links
might make matters worse.
[1] https://www.powermapper.com/tests/screen-readers/navigation/a-target-blank/
