Skip to content

Conversation

Sachin-chaurasiya
Copy link

@Sachin-chaurasiya Sachin-chaurasiya commented Feb 10, 2022

@keithamus, Please review the PR.

@Sachin-chaurasiya Sachin-chaurasiya requested a review from a team as a code owner February 10, 2022 14:51
@@ -129,6 +129,8 @@ class TextExpander {

this.input.value = beginning + value + remaining

this.input.dispatchEvent(new Event('change', {bubbles: true}))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we please add a test that confirms this behaviour, that we can also use to validate and avoid regressions here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@keithamus , I don't have experience in testing. can you please help me out here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@keithamus, I have added some tests by looking at previous tests.

@Sachin-chaurasiya
Copy link
Author

@keithamus , please review this PR.

Copy link
Contributor

@koddsson koddsson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @Sachin-chaurasiya; the tests run fine for me when I remove the line that dispatches the change event.

Maybe you can add motivation for this change to the PR and double-check that this change event is being dispatched already by the browser.

@@ -129,6 +129,8 @@ class TextExpander {

this.input.value = beginning + value + remaining

this.input.dispatchEvent(new Event('change', {bubbles: true}))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests run green for me with this line removed so either the tests are wrong or the implementation.

@Sachin-chaurasiya
Copy link
Author

Closing this PR as an Issue is different.

@Sachin-chaurasiya Sachin-chaurasiya deleted the input-dispatch-event branch April 3, 2022 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants