prevent harmless error in radio-group when non-radio-button is clicked #1981
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
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
KonnorRogers
approved these changes
Apr 22, 2024
| const radios = this.getAllRadios(); | ||
| const oldValue = this.value; | ||
|
|
||
| if (target.disabled) { | ||
| if (!target || target.disabled) { |
| @@ -129,11 +129,11 @@ export default class SlRadioGroup extends ShoelaceElement implements ShoelaceFor | |||
| } | |||
|
|
|||
| private handleRadioClick(event: MouseEvent) { | |||
| const target = (event.target as HTMLElement).closest<SlRadio | SlRadioButton>('sl-radio, sl-radio-button')!; | |||
| const target = event.target.closest<SlRadio | SlRadioButton>('sl-radio, sl-radio-button')!; | |||
There was a problem hiding this comment.
Seems like removing the as HTMLElement here broke the TypeScript check. Mind adding it back in and see if CI passes?
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.
It is sometimes useful to stick non-radio(-button) elements in radio-group to achieve certain effects. In case the clicked element is not a radio-button or radio, skip the logic (instead of throwing an error reading
disabledofnull).Example:
Actual example screenshot (copy-button is outside of the radio-button so that behaviors of hover effects, tooltips, etc, of the radio-button are not mixed with those of the copy-button (for example we don't want a tooltip from inside the radio-button to also trigger while hovering on the copy-button)):
(its a vertical layout, each rectangle is a radio-button, each one shows a copy-button on hover)
Even with the error, this approach works really well, the sl-radio-group is written in a way that the wrappers don't mess it up. The error is harmless too, everything still works with the error because no logic after the error is necessary (equivalent to returning early and doing nothing).