Note we aren't using flex-with-gap because currently (until we decide to
use the `gap` property) this mixin sets a negative margin that would
move the border of this element to the left.
This way we remove duplication in the HTML.
We're also adding a test checking what happens when users can vote in
order to test the `render?` method we've added.
This way polls look more similar to the way they did when the answers
were buttons instead of checkboxes or radio buttons.
Note the styling is tricky because we need to add a `float` property to
the legend so it's actually inside the fieldset. This forces us to add a
`::before` pseudo-element in order to add margin between the legend and
the first label. Another option would be:
```
legend {
&:has(+ label) {
margin-bottom: calc($line-height / 2);
}
+ label {
clear: $global-left;
}
}
```
But the `:has` pseudo-class isn't universally supported yet, and we'd
still have to add `margin-top` to the first label when it comes after a
`.help-text` element.
Due to the presence of the border, we're increasing the margin between
elements a little bit.
Note that adding a pseudoelement to the label is a consequence of adding
the `float` property to the legend, so we're changing the order of the
code so the styles for `legend` appear before the styles in `label`.
This could be the case when JavaScript is disabled.
Note that, in `Poll/WebVote` we're calling `given_answers` inside a
transaction. Putting this code before the transaction resulted in a test
failing sometimes, probably because of a bug that might be possible to
reproduce by doing simultaneous requests.
With the old interface, there wasn't a clear way to send a blank ballot.
But now that we've got a form, there's an easy way: clicking on "Vote"
while leaving the form blank.
Some of the code was in its own component, while some of the code
remained in the polls/show view.
Note that we're re-structuring the code a little bit, so it's clear that
the "already voted" messages are only shown when users can vote. Also
note that now the `can?` condition involves the existence of a
`current_user` and that the poll is not expired, so we can simplify the
`voted_in_web` condition.
This way it'll be easier to refactor it.
Note there was a system test which tested both the callout and the form
when unverified users visit a poll. We've split this system test in two
component tests.
Our original interface to vote in a poll had a few issues:
* Since there was no button to send the form, it wasn't clear that
selecting an option would automatically store it in the database.
* The interface was almost identical for single-choice questions and
multiple-choice questions, which made it hard to know which type of
question we were answering.
* Adding other type of questions, like open answers, was hard since we
would have to add a different submit button for each answer.
So we're now using radio buttons for single-choice questions and
checkboxes for multiple-choice questions, which are the native controls
designed for these purposes, and a button to send the whole form.
Since we don't have a database table for poll ballots like we have for
budget ballots, we're adding a new `Poll::WebVote` model to manage poll
ballots. We're using WebVote instead of Ballot or Vote because they
could be mistaken with other vote classes.
Note that browsers don't allow removing answers with radio buttons, so
once somebody has voted in a single-choice question, they can't remove
the vote unless they manually edit their HTML. This is the same behavior
we had before commit 7df0e9a96.
As mentioned in c2010f975, we're now adding the `ChangeByZero` rubocop
rule, since we've removed the test that used `and change`.
We were using a `height: $line-height` property for this task. One of
the disadvantages of this approach is that things don't look so great
when the label expands over more than one line.
Back when we added that property, browser support for flex layouts
wasn't that great. Now there's universal support for it, so we can use
it instead.
This way changing it will be easier.
Note we're moving the `legislation-draft-versions-form` class into the
form component itself, which is wat we usually do in components.
This way changing it will be easier.
Note we're changing the name of the HTML class to follow our naming
conventions; the `edit_page` class wasn't used anywhere, so we don't
need to change anything else.
This rule was introduced in RuboCop 1.76.0 to ensure methods ending
in '?' return boolean.
This commit applies suggested renames and code cleanup:
- Renames 'is_active?' to 'active_class' since it returns a string
- Renames 'parsed_value' to 'in_favor?' and 'is_request_active' to end with '?'
for boolean semantics
- Skips false positives like 'save', 'auto_labels' or 'save_requiring_finish_signup',
which are not predicate methods.
Bumps [rubocop](https://github.com/rubocop/rubocop) from 1.71.2 to 1.75.8.
- [Release notes](https://github.com/rubocop/rubocop/releases)
- [Changelog](https://github.com/rubocop/rubocop/blob/master/CHANGELOG.md)
- [Commits](https://github.com/rubocop/rubocop/compare/v1.71.2...v1.75.8)
---
updated-dependencies:
- dependency-name: rubocop
dependency-version: 1.75.8
dependency-type: direct:development
update-type: version-update:semver-minor
...
Notes:
This commit also includes several style and lint fixes required after
updating RuboCop:
- Removed redundant parentheses now detected by improved
'Style/RedundantParentheses' (1.72 and 1.75.3).
- Replaced ternary expressions with logical OR when the ternary was
returning 'true', as flagged by 'Style/RedundantCondition' (1.73).
- Adjusted block variables to resolve new 'Lint/ShadowingOuterLocalVariable'
offenses (1.75), helping avoid future conflicts during upgrades with
'rails app:updates'
Signed-off-by: dependabot[bot] <support@github.com>
We were using an <a> tag wrapping the whole content of the banner in
order to make the whole banner clickable. However, that made the text of
the link less concise, affecting people using screen readers. So,
instead, we're using the `card` mixin, which we introduced in commit
f285dfcba.
We're making this change now because the HTML5 Sanitizer that we're
about to enable in the next commit was handling the whitespace inside
the banner differently, causing one test to fail, and we didn't find a
different way to fix it.
This way the fields are easier to use, and we can get rid of the
placeholders.
Note we're simplifying the `answer_result_value` in order to make it
easier to return `0` instead of `nil` when the field is empty.
Also note there's a small change of behavior here; previously, these
fields were empty by default, and now their value is `0` by default, so
blindly clicking the "Save" button would send `0` instead of an empty
value. I don't think it's a big deal, though, but we need to keep that
in mind.
Back when we added all the missing labels (changes that we merged in
commit c34cab282), we forgot about fields which had placeholdes, since
Axe doesn't report an error when there are placeholders but there aren't
labels.
In this case, we were using an invalid <label> tag for the question
options, and <h3> tags as labels for the votes.
Using standard labels solves the issue.
Saying that we're supposed to introduce a descriptive title in a field
labelled as "Title" is redundant. Besides, the text of the placeholder
was barely distinguishable, making it harder to fill in the form.
When using a link, people using screen readers might think they're going
to a new page where the password is going to be shown. With a button,
they get a better idea about what to expect.
Furthermore, with a button, we can use the `aria-pressed` attribute to
indicate whether the password is currently being shown.
We were using an icon for this link, but people who can't see the icon
couldn't know what the link was about. Axe was reporting the following
accessibility error:
```
link-name: Links must have discernible text (serious)
https://dequeuniversity.com/rules/axe/4.10/link-name?application=axeAPI
The following 1 node violate this rule:
Selector: .show-password
HTML: <a href="#" class="show-password">
<span class="icon-eye"></span>
</a>
Fix all of the following:
- Element is in tab order and does not have accessible text
Fix any of the following:
- Element does not have text that is visible to screen readers
- aria-label attribute does not exist or is empty
- aria-labelledby attribute does not exist, references elements
that do not exist or references elements that are empty
- Element has no title attribute
```
This happened when previewing banners in the "new banner form", which
might cause accessibility issues when people access the list of links on
the page.
We were getting the following accessibility error:
```
link-name: Links must have discernible text (serious)
https://dequeuniversity.com/rules/axe/4.9/link-name?application=axeAPI
The following node violate this rule:
Selector: a[href$="new"]
HTML: <a href="/admin/banners/new"><h2></h2><h3></h3></a>
Fix all of the following:
- Element is in tab order and does not have accessible text
Fix any of the following:
- Element does not have text that is visible to screen readers
- aria-label attribute does not exist or is empty
- aria-labelledby attribute does not exist, references elements that
do not exist or references elements that
- Element has no title attribute
```
People using screen readers might have a hard time knowing what a
progressbar is about unless we provide a label for it. Axe was reporting
failures like:
```
aria-progressbar-name: ARIA progressbar nodes must have an accessible
name (serious)
https://dequeuniversity.com/rules/axe/4.10/aria-progressbar-name?application=axeAPI
The following 1 node violate this rule:
Selector: .progress
HTML: <div class="progress" role="progressbar" tabindex="0"
aria-valuenow="0.0" aria-valuemin="0" aria-valuemax="100">
<div class="progress-meter" style="width: 0.0%"></div>
</div>
Fix any of the following:
- aria-label attribute does not exist or is empty
- aria-labelledby attribute does not exist, references
elements that do not exist or references elements that are empty
- Element has no title attribute
```
Note that, in the case of the ballot progressbar, it's easier to use
`aria-labelledby`, while in other place it's easier to use `aria-label`,
so we using the easier solution in each scenario.
We can extract a method to reduce the amount of ERB code and remove the
duplication in the link texts. We also make the list consistent; now we
always use a <strong> tag in the group name, no matter whether there are
many groups or only one.
The original implementation (which was never merged) had a `<select>`
field for the switch, which offered accessibility issues. So I came up
with a very bad idea, which was emulating the look and feel of a select
field while making it more accessible for keyboard users.
This approach is inconvenient because we were using a bunch of ARIA
roles to do the same thing that can be done with a list of links, going
against the first rule of ARIA, which is:
> "Don’t use ARIA if you can achieve the same semantics with a native
> HTML element or attribute
Not only that, but the control was confusing for people using mobile
phones (select fields don't behave the same way), and we were using
*invalid* ARIA roles in this situation, leading Axe to report a critical
accessibility error:
```
aria-required-children: Certain ARIA roles must contain particular
children (critical)
https://dequeuniversity.com/rules/axe/4.10/aria-required-children?application=axeAPI
The following 1 node violate this rule:
Selector: ul[data-dropdown-menu="edw1i2-dropdown-menu"]
HTML: <ul class="dropdown menu" wnenu="edw1i2-dropdown-menu"
data-disable-hover="true" op="true" role="menubar">
Fix any of the following:
- Element has children which are not allowed: button[tabindex]
```
So, at least for now, we're using a simple list of links. We might style
it in the future if we find ways to make usability improvements, but,
for now, it does the job, and it does it better than the custom control
we were using.
This way we can move some system tests to component tests and stop
creating records after starting the browser with a `visit`.
We could also split the system test in two, but since these tests
aren't checking any user interactions, moving the to component tests we
check the same things while making the tests faster.
Since the partial was using an instance variable, we're passing it to
the component. We're naming it `voter_user` instead of `user` because
passing something named `user` could make us think that we're passing
the `current_user`. I wasn't sure about naming it `voter` because it's a
`User` record and not a `Poll::Voter` record, but naming it `voter`
would definitely be an option.
We removed the link to this page in commit 83e8d6035 because poll
questions don't really make sense without a poll.
However, this page also contained information about successful
proposals, which might be interesting so administrators don't have to
navigate to the public area in order to find and create questions based
on successful proposals.
So we're keeping the part about successful proposals and linking it from
the proposals part of the admin area.
Note we're using translation keys like `successful_proposals_tab`, which
don't make sense anymore, for the successful proposals. We're doing so
because we've already got translations for these keys and, if we renamed
them, we'd lose the existing translations and our translators would have
to add them again.
Also note we're changing one poll question test a little bit so we
create the question from a successful proposal using the new page. There
are other tests checking how to create a question from the
admin/proposals#show action and other tests checking what happens when
accessing a successful proposal in the admin section, so we don't lose
any test coverage by changing an existing test instead of adding a new
one.
Finally, note that we've removing the `search` method in poll question
because we no longer use it. This currently makes the
`author_visible_name` database column useless; we aren't removing it
right now because we don't want to risk a possible data loss in a patch
release (we're about to release version 2.3.1), but we might remove it
in the future.
We had inconsistent indentation in many places. Now we're fixing them
and adding a linter to our CI so we don't accidentally introduce
inconsistent indentations again.
