Add buttons to check/uncheck all locales #5488
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
javierm
changed the title
javierm
force-pushed
the
check_all_none
branch
2 times, most recently
from
9ab72c4
to
4f1adec
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| page.find("li:last-child") do |check_none| | ||
| expect(check_none).to have_button "Select none" | ||
| expect(check_none).to have_css "button[type='button'][data-field-name='proposal_ids[]'][data-check-none]" |
There was a problem hiding this comment.
Layout/LineLength: Line is too long. [111/110] (https://rubystyle.guide#max-line-length)
Since this is already a component, we can use the `header` method without much refactoring.
Since we were on the "Pending review" filter, and there were no records pending review, the code checking all checkboxes were checked/unchecked didn't test anything because there were no checkboxes on the page. So we're clicking on the "All" filter first.
Since we don't usually style HTML classes starting with `js-`, we're renaming it, so it's consistent with the `CheckAllNone` name used in the `check_all_none.js` file.
People using screen readers usually expect links to take them somewhere else in the page on to a different page, while they expect buttons to change something on the page. Since we're in the latter scenario, using a button is more accessible. It's also more natural; with a button, we don't need to provide `#` as the URL or stop the default event when the button is clicked. And, unlike links, buttons can be activated with either the space or the enter key. Finally, clicking a link pointing to `#` with the middle mouse button opens a useless new tab, while buttons do nothing in this case. Now that we only have one "All" link on the page, we no longer need to specify which "All" link we're clicking, so we're simplifying the code doing so.
Since they're related, we're making them part of the same list. Instead of finding a way to have the `Select` prefix they had as a label for the list, we're including the "prefix" they had inside their texts, so the text of a button doesn't need any additional context.
The float property was removed in commit b71c61e, but then it was added again in commit 4a6313f. It might have been necessary to do so back then because we had a `select` field instead of the links to set the order, but now, instead of making them float on the left and then make the next element clear the floats, we can do nothing and obtain the same results.
This way we can use SCSS syntax here, like we do everywhere else.
These buttons only work without JavaScript, so we shouldn't show them in this case. I was wondering whether we should use the `hidden` HTML attribute so these buttons don't show up when stylesheets haven't loaded either. Not doing so because we already have a stylesheet for the <noscript> scenario. We might change our minds regarding how to handle these styles in the future.
Although most Consul Democracy installations will only have a few available languages using `config.i18n.available_locales`, there's a chance some installation will keep every language as available and will enable the desired ones using the admin interface. In these cases, enabling (or disabling) every language would be tedious, particularly when casually experimenting in a staging environment or while using the official Consul Democracy demo. So we're adding buttons to simplify the process. Since some installations might have only a couple of available languages, and in this case these buttons would be pretty much useless, we're only showing them when there are many languages available.
We had an inconsistency where most stylesheets associated to a component would have the same relative path as their component, so if we had a component in `app/components/admin/whatever`, its associated stylesheet would be in `app/assets/stylesheets/admin/whatever`. There was one exception to this rule: stylesheets for components in `app/components/shared/` were placed in `app/assets/stylesheets/`. The reason was that we thought "well... if they're in the root folder, they're shared". However, this is confusing because in the root folder there are also stylesheets that aren't associated to a component. So we're creating the `app/assets/stylesheets/shared/` folder. This also means we don't have to manually add every stylesheet in this folder the the `application.scss` file. We aren't the same for JavaScript files because with JavaScript we still don't have a clear association between JavaScript files and components. Only a couple of them (`advanced_search.js` and `check_all_none.js`) would be good candidates, and the one for the advanced search form doesn't even use the `.advanced-search-form` selector that we use in the CSS file.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
References
Objectives
Visual Changes
Before these changes
After these changes