We lost focus styles on certain controls in commit 4dad04ae3, since we
were applying a border with a rule which had more precedence than the
rule of border on focus.
We were using a focus outline on links, but weren't doing the same for
buttons. Since sometimes browsers use a default outline which is barely
visible, this was very disorienting when browsing using the keyboard; we
were navigating through links that clearly indicated where the keyboard
focus was, and when reaching a button suddenly we had this almost
imperceptible feedback. Even if I'm used to it, my first reaction is
always "where did the focus go?" until I realize it's now on a button.
This is even more confusing because we've got buttons looking like links
and links looking like buttons.
Note that in the rules for the `:focus` styles we're including buttons
and the `[type="button"]` attribute. This seems redundant since those
styles are already covered by the `button` selector. However, Foundation
adds styles to buttons with the `[type]` attribute. Since the attribute
selector has precedence over the tag selector, we need to use the
attribute selector as well in order to override Foundation's styles.
There were no elements matching this selector since commit d679c1eb7 and
these styles were completely ignored. I'm re-adding the ones with make
sense in my humble opinion. I'm not adding top and bottom paddings since
they affect the way the height of the element is calculated, and am not
sure about the intention behind setting the height property.
It was broken since we fixed issues with other image fields in commit
394a94cbf, because there we added a fieldset for the image fields, and
so they inherited the styles in budget phases fields (these styles were
added before it was possible to attach an image to a phase).
When users see a label saying "With the text" and an input field, they
don't usually need a placeholder saying "Write the text". On the
contrary, this text adds noise and is hard to read due to the low
contrast between the color of the placeholder and the color of the
field, making the text an unnecessary distraction.
User testing has shown this filter isn't really useful and sometimes
makes users wonder what it's about. This is particularly true in CONSUL
installations which don't change the default values (most of them),
since users will see a filter with options like "Official position 1".
We were using helper methods inside the model; we might as well include
them in the model and use them from anywhere else.
Note we're using a different logic for images and documents methods.
That's because for images the logic was defined in the helper methods,
but for documents the logic is defined in the Documentable concern. In
the past, different documentable classes allowed different content
types, while imageable classes have always allowed the same content
types.
I'm not sure which method is better; for now, I'm leaving it the way it
was (except for the fact that we're removing the helper methods).
The same way it's done for images.
We were converting the number of megabytes to bytes and then converting
it to megabytes again. Instead, we can leave it as it is and only
convert it to bytes when necessary (only one place).
The `sign_in(nil)` method was a bit hard to understand IMHO. After all,
in controller and system tests we don't have to specify no user is
signed in; that's the default behavior.
So we're making it the default behavior for component tests as well.
The chances of an unpublished proposal appearing on the homepage was
very low because only the proposals with the most votes appear there and
unpublished proposals don't have any votes. However, it was technically
possible on new sites where only a few proposals had been created.
Users were unable to reset a translation to its original value after
updating it because we weren't storing anything in the database in that
case.
I've considered deleting the existing translation when this happens. I'm
not sure about which approach is the better one, so I'm using the less
destructive one.
I think we could have already done so when upgrading Ruby to version
2.6.x (which also included the Bundler gem), but since we didn't, now
that we've upgraded to Bundler 2.x it's probably a good moment.
We were getting hundreds of "warning: URI.escape is obsolete" messages.
So we're using `URI::DEFAULT_PARSER.escape` instead.
IMHO it's OK to add this monkey-patch because we're replacing Paperclip
with Active Storage, and when we finish with that we'll delete this
file.
We're not adding the rule because it would apply the current line length
rule of 110 characters per line. We still haven't decided whether we'll
keep that rule or make lines shorter so they're easier to read,
particularly when vertically splitting the editor window.
So, for now, I'm applying the rule to lines which are about 90
characters long.
We forgot to use it in one place, and we've found out other institutions
using CONSUL whose developers aren't so familiar with Ruby also break
this rule, so it might be better to add it explicitly.
