New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[quill] getSelection can return undefined #28736

Merged
merged 1 commit into from Sep 10, 2018

Conversation

Projects
None yet
4 participants
@szhu
Contributor

szhu commented Sep 9, 2018

I think my change is correct but I'm having trouble running the test. Questions bolded below. Thank you!

  • Use a meaningful title for the pull request. Include the name of the package modified.
  • Test the change in your own code. (Compile and run.)
    I can't figure out how to test this. Can someone let me know what command to run?
  • Add or edit tests to reflect the change. (Run with npm test.)
    No tests added, there is already a test for getSelection. It should have errored previously but tsc is not strict enough to do so, so I'm not sure how to add a regression test for this.
  • Follow the advice from the readme.
    I get missing script: tsc even after npm install. I'm not sure what to do.
  • Avoid common mistakes.
  • Run npm run lint package-name (or tsc if no tslint.json is present).

Select one of these and delete the others:

If changing an existing definition:

  • Provide a URL to documentation or source code which provides context for the suggested changes: https://quilljs.com/docs/api/#getselection
  • Increase the version number in the header if appropriate.
    Should I?
  • If you are making substantial changes, consider adding a tslint.json containing { "extends": "dtslint/dt.json" }.
@typescript-bot

This comment has been minimized.

Show comment
Hide comment
@typescript-bot

typescript-bot Sep 9, 2018

@szhu Thank you for submitting this PR!

🔔 @sumitkm @guillaume-ro-fr @43081j @AnielloFalcone @mhamri - please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

If no reviewer appears after a week, a DefinitelyTyped maintainer will review the PR instead.

typescript-bot commented Sep 9, 2018

@szhu Thank you for submitting this PR!

🔔 @sumitkm @guillaume-ro-fr @43081j @AnielloFalcone @mhamri - please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

If no reviewer appears after a week, a DefinitelyTyped maintainer will review the PR instead.

@43081j

43081j approved these changes Sep 9, 2018

would be nice to have a test but i agree its kinda taken care of by the existing tests.

👍

@typescript-bot typescript-bot moved this from Waiting for Reviewers to Check and Merge in Pull Request Status Board Sep 9, 2018

@typescript-bot

This comment has been minimized.

Show comment
Hide comment
@typescript-bot

typescript-bot Sep 9, 2018

A definition owner has approved this PR ⭐️. A maintainer will merge this PR shortly. If it shouldn't be merged yet, please leave a comment saying so and we'll wait. Thank you for your contribution to DefinitelyTyped!

typescript-bot commented Sep 9, 2018

A definition owner has approved this PR ⭐️. A maintainer will merge this PR shortly. If it shouldn't be merged yet, please leave a comment saying so and we'll wait. Thank you for your contribution to DefinitelyTyped!

@RyanCavanaugh RyanCavanaugh merged commit 2b62080 into DefinitelyTyped:master Sep 10, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@typescript-bot typescript-bot removed this from Check and Merge in Pull Request Status Board Sep 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment