Add open attribute to component wrapper #4074
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
AshGDS
reviewed
Jun 24, 2024
There was a problem hiding this comment.
Looks good 👍
According to MDN, setting open to false will open the details component:
Note: You have to remove this attribute entirely to make the details hidden. open="false" makes the details visible because this attribute is Boolean.
Unless I'm misreading the code, does this need to be reflected in this PR? I can see that in your "can set an open attribute, overriding a passed value" test the open attribute is nil after you set the value to false, so maybe you've already accounted for this?
|
Good point @AshGDS thanks. Have added an explicit test to check this, do you think that covers this? |
- adds the ability to pass an open attribute to the component wrapper helper - needed for the details component, where open can be passed as true to have the details component expanded by default
|
Thanks @AshGDS |
Merged
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
Why
Originally done as part of #4068 but that PR is huge enough without this additional change, so I'm separating them out.
Visual Changes
None.
Trello card: https://trello.com/c/rNPqwDIr/188-remove-ua-code-from-components-gem