The `alt` attribute is mandatory in image tags. In this case, we're
leaving it empty because we also display text showing whether comments
are made by administrators, moderators or organizations.
The initialjs-rails gem hasn't been maintained for years, and it
currently requires `railties < 7.0`, meaning we can't upgrade to Rails 7
while we depend on it.
Since the code in the gem is simple, and we were already rewriting its
most complex part (generating a background color), we can implement the
same code, only we're using Ruby instead of JavaScript. This way, the
avatars will be shown on browsers without JavaScript as well. Since
we're adding a component test that checks SVG images are displayed even
without JavaScript, we no longer need the test that checked images were
displayed after AJAX requests.
Now the tests show the user experience better; people don't care about
the internal name used to select the initial (which is what we were
checking); they care about the initial actually displayed.
Note initialjs generated an <img> tag using a `src="data:image/svg+xml;`
attribute. We're generating an <svg> tag instead, because it's easier.
For this reason, we need to change the code slightly, giving the <svg>
tag the `img` role and using `aria-label` so its contents won't be read
aloud by screen readers. We could give it a `presentation` role instead
and forget about `aria-label`, but then screen readers would read the
text anyway (or, at least, some of them would).
These images are always displayed next to a username, meaning people
using screen readers were hearing the same username twice in a row.
Even though we're about to replace the initialjs gem, we're making this
change in case so we've got one more test and we can check everything
keeps working after replacing the gem.
We were using generic names like `args` and `options` which don't really
add anything to `*` or `**` because Ruby required us to.
That's no longer the case in Ruby 3.2, so we can simplify the code a
bit.
Note that the `budget` parameter was added to the `delete_path` method
so it works in the tests; on production, it worked because this
component is only rendered on pages which already have the `budget`
parameter.
Co-authored-by: Javi Martín <javim@elretirao.net>
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.
