Skip to content
This repository has been archived by the owner on Jul 25, 2024. It is now read-only.

[#PAB-508]: add scrollable checkboxes #51

Merged
merged 1 commit into from
Jul 31, 2023
Merged

Conversation

Joel-Pearce
Copy link
Contributor

Placed the dropdown checkboxes inside a scrollable frame as per ticket: https://trello.com/c/rcDW6WTF/508-adding-ability-to-scroll-in-filters

  • Unit tests and other appropriate tests added or updated
  • README and other documentation has been updated / added (if needed)
  • Commit messages are meaningful and follow good commit message guidelines (e.g. "FS-XXXX: Add margin to nav items preventing overlapping of logo")
image

@Joel-Pearce Joel-Pearce temporarily deployed to test July 31, 2023 09:06 — with GitHub Actions Inactive
Copy link
Contributor

@bobbi-hbrown bobbi-hbrown left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code changes look good from my testing, is there a specific reason for wrapping the checkboxes in a new div rather than attaching the custom CSS to the original govuk-checkboxes? No worries either way I was just wondering :)

@Joel-Pearce
Copy link
Contributor Author

The code changes look good from my testing, is there a specific reason for wrapping the checkboxes in a new div rather than attaching the custom CSS to the original govuk-checkboxes? No worries either way I was just wondering :)

The class overwrite looked a little more fiddly than just wrapping things in a div (for some of the macros it's not as simples as setting classes='new_class'), if that's what you mean. Might have to do class overwrites for more complicated custom CSS changes though.

@Joel-Pearce Joel-Pearce merged commit 9f6374c into main Jul 31, 2023
2 checks passed
@Joel-Pearce Joel-Pearce deleted the feature/add-scrollbars branch July 31, 2023 10:02
{{ item['name'] }}
</label>
<div class="scrollable-checkboxes">
<div class="govuk-checkboxes" data-module="govuk-checkboxes">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have we tried adding this class as an additional class on the div below?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I'll try that, and refactor it if successful.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants