Commit Graph

2 Commits

Author SHA1 Message Date
Javi Martín
2b962f2789 Use a <main> tag on every page
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.
2024-03-23 00:35:43 +01:00
Javi Martín
7bb7548d00 Respond with 403 when features are disabled
When administrators disabled features and users tried to access them
with the browser, we were responding with a 500 "Internal Server Error"
page, which in my humble opinion was incorrect. There was no error at
all; the server worked exactly as expected.

I think a 403 "Forbidden" code is better; since that feature is
disabled, we're refusing to let users access it.

We could also respond with a 404 "Not found", although I wonder whether
that'll be confusing when administrators temporarily or accidentally
disable a feature.

A similar thing might happen if we responded with a 410 "Gone" code.
Actually this case might be more confusing since users aren't that
familiar with this code.

In any case, all these options are better than the 500 error.
2021-06-16 20:45:15 +02:00