Refactor the accordion component code #2797
Merged
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
- I think these were previously used when the accordion had options to pass text from Rails to the JS, in order to do translations - but they don't appear to be used anywhere
- separate class names from element selectors - rename class name variables to distinguish them - include '.' in selector variables instead of doing it multiple times throughout the code
- class was being added on component JS initialisation - appears to only be needed for a test to pass - removing as this is not how we should be testing this code, the JS is currently not fully tested but this is on our backlog to address
We're working on that at the moment! alphagov/govuk-frontend#1714 Signed, |
|
Brilliant, thanks @querkmachine! |
AshGDS
approved these changes
Jul 14, 2022
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.
What
Refactor some things in the accordion code. Specifically:
.except it's needed every time the variable is usedWhy
I made some unrelated changes in the accordion recently and this was bothering me.
I'm 99.9% certain the variables I've removed aren't used but worth double checking. I think we used to have the ability to pass translated text to the component for the 'show/hide' and 'show/hide all' links (since they're created by JS, which can't access translations from Rails) but don't have that since now most of the accordion component JS is managed by code from
govuk-frontend. At some point it might be good to think about how to bring that functionality back.Visual Changes
None.