Since we're simplifying the main method, we can use a view file instead
of the `call` method. This way we make the code more consistent with the
rest of our components, since we always use a separate file.
Doing so generates an extra newline at the end of the generated HTML, so
we need to change a couple of tests a little bit.
This way it's easier to refactor it and/or change it.
Back in commit c156621a4 I wrote:
> Generally speaking, I'm not a big fan of helpers, but there are
> methods which IMHO qualify as helpers when (...) many Rails helpers,
> like `tag`, follow these principles.
It's time to modify these criteria a little bit. In some situations,
it's great to have a helper method so it can be easily used in view
(like `link_to`). However, from the maintenance point of view, helper
methods are usually messy because extracting methods requires making
sure there isn't another helper method with that name.
So we can use the best part of these worlds and provide a helper so it
can be easily called from the view, but internally make that helper
render a component and enjoy the advantages associated with using an
isolated Ruby class.
Since forms are landmarks, screen reader users might navigate to the
form. But then they were going to find an empty form with no way to
toggle it.
Moving the button inside the form means screen reader users navigating
to the form will find the button to toggle it.
It also helps us simplifying the code; there's no need to use
data-attributes to communicate whether the form should be visible since
now we can easily use the button `aria-expanded` attribute.
We could further simplify the JavaScript if we used a CSS rule to
show/hide the form fields based on the toggle button `aria-expanded`
attribute. However, implementing the "slide" animation we use when
toggling the form with CSS is difficult and unreliable.
We were using the form and then showing it with JavaScript when advanced
search terms were present. Now we hide it with JavaScript when no
advanced search are present. This means users without JavaScript
(including users with JavaScript enabled but bad internet connections
preventing the JavaScript to load) can now access the form.
The other main difference between the two versions is the way the form
flashes while JavaScript is loading.
Previously, the form would always be hidden when no terms had been
introduced. However, when these terms were present, after submitting the
form it would briefly be hidden and then shown again.
Now the opposite happens. When advanced search terms are present, the
form is shown at all times. However, when they aren't, the form is
briefly shown before it disappears.
Here the previous behavior is arguably better because most of the time
these terms will not be present.
So basically we're significantly improving the experience of some users
at the cost of slightly worsen the experience of other users.
We're also hiding the button to show the form when JavaScript is
disabled, since in this scenario it's useless. We're using the `hidden`
attribute so hidden buttons can be detected in CSS.
Checking the `style` attribute fails in JavaScript tests because the
browser converts colors to the `rgb()` format.
So we're testing the generated HTML in a component test while
simplifying the system test.
Now it's easier to change the investments filter. Previously we had to
go back to the budget index page, change the filter there, and then
select one heading.
Now the links to change the current filter in the budget index page
aren't needed anymore.
Now the banner component accepts either a banner or a section and loads
the banner if it's a section, so we don't have to add the `@banners`
variable in several controllers.
