va-statment-of-truth: make event payloads easier to use #1147
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
6 tasks
ataker
approved these changes
May 15, 2024
jamigibbs
reviewed
May 15, 2024
| @@ -161,7 +167,7 @@ export class VaStatementOfTruth { | |||
| value={inputValue} | |||
| message-aria-describedby={inputMessageAriaDescribedby} | |||
| required | |||
| onInput={this.handleInputChange} | |||
| onVaTextInputChange={this.handleInputChange} | |||
There was a problem hiding this comment.
I'm not sure that I totally understand why we need two "change" events for va-text-input now: onInput and onVaTextInputChange. Can you help me understand why we couldn't continue to use the nativeonInput here and avoid having to modify va-text-input?
There was a problem hiding this comment.
@jamigibbs - you're right! I refactored to avoid using a custom event.
| /** | ||
| * The event emitted when the input value of the va-text-input element changes. | ||
| */ | ||
| @Event() vaTextInputChange: EventEmitter; |
There was a problem hiding this comment.
Follow on my previous comment, I'm curious if instead of a new custom event here in va-text-input, it could instead be in va-statement-of-truth so that we can keep using the native onInput event?
There was a problem hiding this comment.
@jamigibbs - see above!
Andrew565
approved these changes
May 15, 2024
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.
Chromatic
https://2783-va-statement-truth-bug--65a6e2ed2314f7b8f98609d8.chromatic.com
Description
This PR addresses a bug in which the events payloads for the
vaInputChangeandvaCheckBoxChangewere difficult or impossible to make use of in an event handler. Now the payloads are at the top level of the event objects.Closes 2783
QA Checklist
Screenshots
vaInputChangeevent before:vaInputChangeevent aftervaCheckBoxChangeevent beforevaCheckboxChangeevent afterAcceptance criteria
Definition of done