Commit Graph

12 Commits

Author SHA1 Message Date
Javi Martín
469b39ffa3 Add and apply Style/RedundantInterpolation rule
Somehow I thought we already had this rule; must have forgotten to
actually add it.
2021-08-09 21:37:04 +02:00
taitus
ee66c4182a Return error when the related content already exists
It looks like this bug was introduced in commit 7ca55c4. Before that
commit, a message saying "You added a new related content" was shown,
but (as expected) the related content wasn't added twice.

We now check this case and add a slightly clearer message.
2021-06-25 17:15:34 +02:00
Javi Martín
89a06ea6ef Fix related content with custom URLs
Some CONSUL installations might want to customize their URLs. For
instance, Spanish institutions might want to use "/propuestas" instead
of "/proposals". In that case, it would be impossible to add proposals
as related content because the related contents controller assumed the
name of the model was part of the URL.

Using `recognize_path` instead of manually analyzing the URL solves the
issue.

Now that we don't call the `constantize` method on an empty string as we
previously did, we can be more specific in the `rescue` block and point
out that the only exception we expect is the one where users enter a
route which isn't recognized.
2021-06-23 23:28:35 +02:00
Javi Martín
f298a308c2 Show actions to score related content to everyone
We were only showing these actions to users with small screens and to
mouse users on hover. Keyboard users or users with a touch screen of a
medium or large size could never find out the actions were there.
2021-06-23 23:06:24 +02:00
Javi Martín
49e0138cd1 Fix label in related content form
It didn't have a `for` attribute and so it wasn't correctly associated
with its input. That means clicking on / touching the label didn't have
the effect of focusing on the field, and screen readers wouldn't
announce the label.
2021-06-23 23:06:24 +02:00
Javi Martín
85215e7e97 Fix aria-expanded value in related content button
The button was announced as expanded when the form was hidden and as
collapsed when the form was shown.

This is because Foundation sets the expanded attribute based on whether
the class to toggle already exists. Since initially the form had the
"hide" class and the button toggled that class, Foundation checked that
the class was already present and so set the button as expanded.

So we're changing the toggler class for a class we don't use at all,
just so Foundation initiall sets `aria-expanded=false` and then changes
it to `aria-expanded=true` after the button is clicked. Then we're
ignoring this class completely and are styling this form with CSS
instead.

We could also use a toggler class like "visible" and write something
like:

```
.add-related-content + form:not(.visible) {
  display: none;
}
```

However, using ARIA attributes is more robust as it guarantees the
styles will always be in sync with what screen reader users experience.

And we could also remove all the Foundation toggler functionality and
use our own JavaScript to handle the button state. We might do so in the
future.
2021-06-23 23:06:24 +02:00
Javi Martín
f1221e9c1c Make relationable test expectations consistent
We're removing the parenthesis after page expectations (as we do almost
everywhere) and we're replacing `have_selector` with `have_css`, since
on that file we were using `have_css` everywhere except in one place.
2021-06-23 23:06:24 +02:00
Javi Martín
8a697216d1 Fix invalid HTML in related content button
A button cannot be inside an anchor tag, and it might confuse some
browsers or screen readers.

We're also making it clear in the tests that the intention is to use a
button there by using `click_button` instead of `click_on` since the
latter also clicks on links.
2021-06-22 22:32:08 +02:00
Javi Martín
a7664ad817 Query the database before visiting a page in tests
We can assign query results to variables and so we avoid querying the
database after starting the browser.
2021-04-16 14:33:26 +02:00
Javi Martín
92ddcb7aef Use JavaScript in system tests by default
JavaScript is used by about 98% of web users, so by testing without it
enabled, we're only testing that the application works for a very
reduced number of users.

We proceeded this way in the past because CONSUL started using Rails 4.2
and truncating the database between JavaScript tests with database
cleaner, which made these tests terribly slow.

When we upgraded to Rails 5.1 and introduced system tests, we started
using database transactions in JavaScript tests, making these tests much
faster. So now we can use JavaScript tests everywhere without critically
slowing down our test suite.
2021-04-07 14:41:06 +02:00
Javi Martín
6088334dbf Remove redundant visibility matcher usages
By default, Capybara only finds visible elements, so adding the
`visible: true` option is usually redundant.

We were using it sometimes to make it an obvious contrast with another
test using `visible: false`. However, from the user's perspective, we
don't care whether the element has been removed from the DOM or has been
hidden, so we can just test that the visible selector can't be found.

Besides, using `visible: false` means the test will also pass if the
element is present and visible. However, we want the test to fail if the
element is visible. That's why a couple of JavaScript-dependant tests
were passing even when JavaScript was disabled.
2020-10-25 14:23:53 +01:00
Javi Martín
9427f01442 Use system specs instead of feature specs
We get rid of database cleaner, and JavaScript tests are faster because
between tests we now rollback transactions instead of truncating the
database.
2020-04-24 15:43:54 +02:00