Use option-select vs checkboxes for granting permissions #2424
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
floehopper
force-pushed
the
use-option-select-vs-checkboxes-for-granting-permissions
branch
3 times, most recently
from
c776419 to
a12bc15
Compare
floehopper
changed the title
This should have been changed in this earlier commit [1]. [1]: 735fd59
Trello: https://trello.com/c/SfIc6Q2q The main motivation for these changes is the Licensing app which has a large number of permissions. The recent changes [1,2] which moved the UI for granting permissions to the GOV.UK Design System have made managing the permissions for the Licensing unmanageable. This replaces the accordion component [3] where each item of the accordion was a checkboxes component [4] (i.e. a set of checkboxes) with a list of option-select components [5] (each of which has a set of checkboxes within it). We've made this change to *both* the new invitation ("Create new user") page and the new batch invitation permissions page. The option-select component gives us something similar to the show/hide toggle which was previously provided by the accordion which is why it no longer makes sense to use the accordion. Also it makes the permissions section more compact which is no bad thing given how long it is! Most importantly the option-select component gives us the option to enable a search filter for the permissions for a given app. I plan to make use of this in a subsequent commit. This will address the primary motivation which is the unmanagability of the Licensing app permissions. Note that I've set the `key` attribute of the option-select components to "not-used" for now, because it's a required attribute but the name of the checkboxes is being derived from each individual option hash. I plan to address this in a subsequent commit. The use of the option-select component isn't really what it was intended for in that it doesn't *control* a list of things. However, I don't think it's too far away and it gives us a much better UI. [1]: #2361 [2]: #2412 [3]: https://components.publishing.service.gov.uk/component-guide/accordion [4]: https://components.publishing.service.gov.uk/component-guide/checkboxes [5]: https://components.publishing.service.gov.uk/component-guide/option_select
We don't really need to set the `id` attribute for each checkbox in `UsersHelper#options_for_permission_option_select` - the checkboxes component within the option-select component will generate unique `id` values automatically and we don't need to reference them anywhere. This simplifies the code somewhat.
Set the name of the permission checkboxes at the option-select component level (via its `key` attribute) rather than having to set it for each of its options in `UsersHelper#options_for_permission_option_select`. This simplifies the code somewhat.
Enable the search filter for the option-select component for applications which have more than 4 permissions which is roughly the point that a scrollbar appears. This addresses the primary motivation of this PR which is to improve the managability of the Licensing app permissions.
Set each option-select component to open on load only if any of its permissions are selected. At the moment this is only relevant to the new invitation page when validation errors have occurred and the form has been re-rendered. The new batch invitation permissions page currently never has any validation errors, so we can just hard-code the value of `closed_on_load` to `true`. This behaviour is very similar to what we did on the users index page. Since this behaviour is implemented in JS, it's not possible to test it in the controller test and it doesn't seem important enough to add an integration test for it.
I want to be able to use this in the new invitation page and the new batch invitation permissions page. Moving it to a more generic namespace will make that easier.
This change ensures any checked permission checkboxes are displayed at the top of the list of checkboxes for a given application option-select. At the moment this is only relevant to the new invitation page when validation errors have occurred and the form has been re-rendered. Currently the new batch invitation permissions page can never have any validation errors. This behaviour is very similar to what we did on the users index page.
floehopper
force-pushed
the
use-option-select-vs-checkboxes-for-granting-permissions
branch
from
a12bc15 to
7eb14ee
Compare
chrislo
approved these changes
Oct 11, 2023
floehopper
deleted the
use-option-select-vs-checkboxes-for-granting-permissions
branch
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.
Trello: https://trello.com/c/SfIc6Q2q
The main motivation for these changes is the Licensing app which has a large number of permissions. The recent changes which moved the UI for granting permissions to the GOV.UK Design System have made managing the permissions for the Licensing app unmanageable.
This PR replaces the accordion component where each item of the accordion was a checkboxes component (i.e. a set of checkboxes) with a list of option-select components (each of which has a set of checkboxes within it). We've made this change to both the new invitation ("Create new user") page and the new batch invitation permissions page.
The option-select component gives us something similar to the show/hide toggle which was previously provided by the accordion which is why it no longer makes sense to use the accordion. Also it makes the permissions section more compact which is no bad thing given how long it is! Most importantly the option-select component provides a search filter for the permissions for a given app - this is enabled for apps which have more than 4 permissions which is roughly the point that a scrollbar appears. This addresses the primary motivation which is improving the managability of the Licensing app permissions.
We've also set each option-select component to open on load if any of its permissions are selected and moved any checked permission checkboxes to the top of the list. At the moment this is only relevant to the new invitation page when validation errors have occurred and the form has been re-rendered. This behaviour is very similar to what we did on the users index page.
The use of the option-select component isn't really what it was intended for in that it doesn't control a list of things. However, I don't think it's too far away and it gives us a much better UI.
In making this change I've worked out we don't really need to set the
idattribute for each checkbox inUsersHelper#options_for_permission_option_select- the checkboxes component within the option-select component will generate uniqueidvalues automatically and we don't need to reference them anywhere. Also we can set the checkbox name at the option-select component level (via itskeyattribute) rather than having to set it for each of its options inUsersHelper#options_for_permission_option_select. This simplifies the code somewhat.New invitation page
New batch invitation permissions page