When editing/showing a proposal or an investment, the most relevant
information regarding the marker are the coordinates. The title of the
proposal or investment is redundant because we already know the marker
is about that proposal/investment.
There's one problem with this approach, though: when editing a proposal
or an investment, the aria-label of the marker isn't updated
automatically when we move the marker to a different place. This
behaviour will only affect people who use both a screen reader and a
mouse, since keyboard users can't change the position of the marker in
the first place. We'll deal with this issue when we make it possible to
change the position of a marker using the keyboard.
We forgot to do so in commit b896fc4bb. Back then, we said:
> Note that we aren't providing a proper aria-label for markers on the
> map we use in the form to create a proposal or an investment. Adding
> one isn't trivial given the current code, and keyboard users can't add
> a marker in the first place. We'll have to revisit this issue when we
> add keyboard support for this.
However, in the admin section, the marker is already there, so it should
have a label. In this case, we're using the coordinates as label because
it's the most relevant text for the marker in the context of a form. We
could also use "Default map location" instead, but that information is
already present on the page.
Axe was reporting the same accessibility error we mentioned in commit
b896fc4bb in this situation.
This way we remove duplication.
Note that to check whether to render the button to remove a marker,
we're checking whether the map location belongs to a mappable. This
means we're changing the code that renders the map in the "new proposal"
and "new investment" forms so the map location belongs to a proposal or
investment. We're association the map location to a new record because
writing something like:
```
def map_location
proposal.map_location || MapLocation.new(proposal: proposal)
end
```
Would change the `proposal` object because of the way Rails treats
non-persisted `has_one` associations. Although probably safe in this
case, changing an object when rendering a view could have side effects.
Also note that we're changing the HTML ID of the map element from
`admin-map` to `new_map_location` (the latter is returned by the
`dom_id` method). We were only using this ID in tests since commit
289426c1c, so changing it doesn't really affect us.
We were using the same texts twice. For the remove marker label text,
however, we were using the text defined in proposals for both proposals
and investments.
Ideally the translation keys for these texts would go in another
namespace, since they no longer refer to just proposals. However,
renaming the translation keys would mean losing the existing
translations in every language we manage through Crowdin. So we aren't
doing so.
The `m_l` prefix isn't really necessary when we're talking about map
locations, and sometimes when searching the project I think `m_l_`
stands for "machine learning" and get confused.
The "by_authors" scope was the last remaining name from the removed
family of `by_author` scopes. It no longer reflects its purpose: it
simply loads users by IDs.
The "on_budget_investments" scope in Activity has never been used
anywhere in the codebase. It was introduced in commit d9d38482b3
("extends Activity to include Investment valuations") but no references
were ever added.
Instead of removing it, we make use of the scope by adding the missing
"Budget investments" filter to the admin Activity section. This aligns
it with the rest of the activity filters and gives the scope the purpose
it was originally intended for.
The "by_official_level" scope in Proposal is no longer used anywhere in
the code. Its last use was removed in commit 9f1f912d84 ("Remove
official level filter from advanced search").
The "sort_by_most_commented" scope in Debate is no longer used anywhere in
the code. Its last use was removed in commit b89f39bfef ("Removes
unused orders from debates controller")
The "created_by" scope in Proposal is no longer used anywhere in the code.
It was introduced in 77dd604 and its last usage was dropped in commit 64258baf97
("Refactor getting the public activity information").
The "for_render", "sort_by_hot_score" and "sort_by_most_commented"
scopes in Legislation::Proposal are no longer used
anywhere in the code. They were all introduced in commit 335399e571
("Created Legislation Proposals model") and have never been
referenced since.
The "by_geozone_id" scope in Poll is no longer used anywhere in the code.
It was first introduced in commit 20cb044015 ("adds search and filter
for poll questions") and later moved to the Poll model in commit
d024505960 ("moves geozones from poll question to poll in models"),
but has never been referenced since.
The "by_email" scope in VerifiedUser is no longer used anywhere in the
code. Its last occurrence was removed in commit 76daee1fb0 ("removes
unmasked emails and phones in forms").
Back in commit c6f0a3761, we replaced the link to sign out with a
button. However, this button is a child of a data-responsive-menu
dropdown element. Since Foundation adds the `menubar` role to responsive
menus, its children are supposed to have a `menuitem` role. So we're
adding it.
Note we're adding the role with JavaScript because, when JavaScript is
disabled, Foundation won't change the `responsive-menu` role to
`menubar`. So we can't have a `menuitem` in this case.
Axe was reporting the following issue:
```
Found 1 accessibility violation:
1) aria-required-children: Certain ARIA roles must contain
particular children (critical)
https://dequeuniversity.com/rules/axe/4.11/aria-required-children?application=axeAPI
The following 1 node violate this rule:
Selector: .account-menu
HTML: <ul class="account-menu menu dropdown"
data-responsive-menu="medium-dropdown" role="menubar"
data-dropdown-menu="cabp3q-dropdown-menu"
data-mutate="ph8tvp-responsive-menu">
Fix any of the following:
- Element has children which are not allowed: button[tabindex]
```
Axe was reporting an accessibility error:
```
Found 1 accessibility violation:
1) aria-command-name: ARIA commands must have an accessible name
(serious)
https://dequeuniversity.com/rules/axe/4.11/aria-command-name?application=axeAPI
The following 1 node violate this rule:
Selector: .leaflet-marker-icon
HTML: <div class="leaflet-marker-icon map-marker
leaflet-zoom-animated leaflet-interactive"
tabindex="0" role="button">
<div class="map-icon"></div>
</div>
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
```
Using the title of the proposal/investment as the text of the marker is
definitely a good solution when there are several markers on the map.
Not sure whether there's a better option when there's only one marker,
though.
Note that we aren't providing a proper aria-label for markers on the map
we use in the form to create a proposal or an investment. Adding one
isn't trivial given the current code, and keyboard users can't add a
marker in the first place. We'll have to revisit this issue when we add
keyboard support for this.
We're also changing a test to make sure that titles with quotes in their
names don't break the markup due to an invalid aria-label attribute.
This part of the page can be scrolled independently with the mouse, and
so the same should be possible with a keyboard.
Axe was reporting this error:
```
1) scrollable-region-focusable: Scrollable region must have
keyboard access (serious)
https://dequeuniversity.com/rules/axe/4.11/scrollable-region-focusable?application=axeAPI
The following 1 node violate this rule:
Selector: .fullscreen > .markdown-preview.medium-6.small-12
HTML: <div class="small-12 medium-6 column markdown-preview">
Fix any of the following:
- Element should have focusable content
- Element should be focusable
```
According to W3C documentation [1]:
> For compatibility with legacy content, the word-break property also
> supports a deprecated break-word keyword. When specified, this has the
> same effect as word-break: normal and overflow-wrap: anywhere,
> regardless of the actual value of the overflow-wrap property.
This is currently *not* detected by the `property-no-deprecated`
stylelint rule. However, since this rule detects other places where we
should use `overflow-wrap`, we're changing it as part of the same pull
request.
Note that, as of November 2025, 98.22% of the browsers support
`overflow-wrap: anywhere`. We're still making this change because we
only use this property in two specific places and it isn't very likely
that long words will really be a problem with the remaining 1.78%.
By the way, I'm not sure why we use this property in the budget
invesment show paragraphs and not in other similar places. It was
introduced in commit e3878ff43, alongside other changes which don't seem
to be related. When in doubt, we leave things as they were, so we're
doing the same thing here.
[1] https://www.w3.org/TR/css-text-3/#word-break-property
[2] https://caniuse.com/mdn-css_properties_overflow-wrap_anywhere
This way we make it easier to customize this component. In this specific
case, ideally we'd remove some of the CSS as well. For now we're leaving
it as it was for compatibility reasons.
This way it's easier to know that the styles for the `break` HTML class
and the JavaScript for sortable elements (which we shouldn't use, by the
way, because of its accessibility issues) are only used here.
This rule was introduced in stylelint 16.23. Out of the deprecated
properties it detects, we were only using `word-wrap`.
According to the CSS Working Group [1]:
> For legacy reasons, UAs must treat word-wrap as a legacy name alias of
> the overflow-wrap property.
Since there's universal browser support for `overflow-wrap: break-word`
[2], we can safely replace `word-wrap` with `overflow-wrap`.
[1] https://drafts.csswg.org/css-text/#overflow-wrap-property
[2] https://caniuse.com/mdn-css_properties_overflow-wrap_break-word
The button flashed briefly when loading the page. This was even more
obvious when using devices with slow internet connections.
Using CSS instead of JavaScript to hide the menu solves the issue, since
styles are loaded before scripts.
This method was returning a boolean value and caused a
`Naming/PredicateMethod` when upgrading rubocop.
So, instead, we're returning the created line when it was successfully
created, and `nil` when it wasn't.
Having said that, I'm not sure why we added the `.persisted?` back in
commit 3eb22ab7b since as far as I can tell we don't use the return
value for anything. The test added in commit da43e9e2e for this change
passes if we simply return `lines.create(investment: investment)`.
For now I'm leaving the `persisted?` check just in case, but removing it
might be fine.
This method was calling `check_availability`, which returned a boolean
value and caused a `Naming/PredicateMethod` when upgrading rubocop.
So we're changing the logic a little bit to remove the
`check_availability` method and merge the tests of `check_availability`
and `notifiable_available?` (which were almost identical) together.
This way it's more obvious that the method is supposed to return a
boolean. When upgrading rubocop, we get a `Naming/PredicateMethod` error
due to `.find` returning a boolean.
This rule was added in rubocop-rails 2.33.
At first, I wasn't very fond of this rule. It made the code less
readable even if it improved performace in some cases.
Then I realized that in the `Admin::MachineLearning::SettingComponent`
we were using `find_by` when we should be using `find_by!` instead, and
we detected that thanks to this rule.
So, only for that reason, I'm adding this rule, but I'm fine if we
remove it.
We introduced this issue in commit f8faabf7d.
Since this component didn't have any tests (there are system tests for
it, though), we're also adding tests that check that only the right
buttons are rendered when accessing as administrator.
