Budi 9006 allow automation test data modal to select real data to #15425
Conversation
…allow-automation-test-data-modal-to-select-real-data-to
|
QA Wolf here! As you write new code it's important that your test coverage is keeping up. |
| const onChange = event => { | ||
| dispatch("change", event.target.checked) | ||
| const onChange: ChangeEventHandler<HTMLInputElement> = event => { | ||
| dispatch("change", event.currentTarget.checked) |
There was a problem hiding this comment.
currentTarget can differ from target depending on how listeners are set up and if the event is bubbling. In this case they should be the same as the element that triggers the event (target) is the same as the element which the event listener is attached to (currentTarget), as they should both be the input element. But just something to keep in mind in future as this could be a breaking change if we were manually adding the listener at a higher level (like on the full document).
There was a problem hiding this comment.
.target doesn't exist on the event type, which is the only reason I changed it.
There was a problem hiding this comment.
This is sort of what I was alluding to in my comment below about the event types being difficult to work with - target 100% does exist in the real event, so I'm not sure why it doesn't in the type.
| export let disabled = false | ||
| export let readonly = false | ||
| export let size | ||
| export let size: string = "M" |
There was a problem hiding this comment.
You can probably take this further and type this as "S" | "M" | "L". Not sure if XS and XL are valid options here but they might be as well - would need to check the CSS file imported at the top here.
| dispatch("change", event.target.value) | ||
| const onChange = (event: FocusEvent) => { | ||
| const target: HTMLTextAreaElement | null = | ||
| event.target as HTMLTextAreaElement |
There was a problem hiding this comment.
I've found the typing around events to be pretty awful to work with. Target is not actually nullable - it will always exist, so I usually just forcefully cast this to be an element type.
| ): ((...params: Parameters<T>) => Promise<R>) => { | ||
| let queue: (() => Promise<void>)[] = [] | ||
| return (...params: Parameters<T>): Promise<R> => { | ||
| return new Promise<R>((resolve, reject) => { |
There was a problem hiding this comment.
I'm sure this was fun to type 😂 Nice one.
There was a problem hiding this comment.
I was readily admit that I got help from ChatGPT 😅
Description
Describe the problem or feature in addition to a link to the relevant github issues.
Addresses
<Enter the Link to the issue(s) this PR addresses>App Export
Screenshots
If a UI facing feature, a short video of the happy path, and some screenshots of the new functionality.
Launchcontrol
Add a small description in layman's terms of what this PR achieves. This will be used in the release notes.