Skip to content
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

fix: check if event is cancelable before calling preventDefault #132

Merged
merged 4 commits into from
Dec 12, 2022

Conversation

danhobbs75
Copy link
Contributor

Hi Matt,

I've made the one-line change and I did check your automated tests run.

I didn't bother to create a branch. I don't normally bother as I work mostly alone and it's rare I work on code that isn't part of the core and needs to be in the next release anyway. Tell me if you'd rather I created a branch.

I will also confess I have not tested this by building it and using it in place of your npm library. This is down to time and the fact that this change is so simple. I hope you don't mind me not doing that - I would look at how to use it in place of an installed npm dependency for something more complex.

Over to you to accept or to bat back to me then...

Dan

.gitignore Outdated
@@ -45,3 +45,5 @@ projects/angular-resizable-element/package.json
# System Files
.DS_Store
Thumbs.db
package-lock.json
.gitignore
Copy link
Owner

Choose a reason for hiding this comment

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

nit: can you remove the .gitignore from this file as we don't want it to ignore itself 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Soz. Done.

@codecov-commenter
Copy link

codecov-commenter commented Dec 12, 2022

Codecov Report

Merging #132 (ccab3a0) into main (73346e9) will decrease coverage by 0.06%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #132      +/-   ##
==========================================
- Coverage   81.00%   80.93%   -0.07%     
==========================================
  Files           5        5              
  Lines         279      278       -1     
  Branches       58       58              
==========================================
- Hits          226      225       -1     
  Misses         42       42              
  Partials       11       11              
Impacted Files Coverage Δ
...r-resizable-element/src/lib/resizable.directive.ts 82.21% <100.00%> (-0.09%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Owner

@mattlewis92 mattlewis92 left a comment

Choose a reason for hiding this comment

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

LGTM!!

@mattlewis92 mattlewis92 changed the title fix: check if event is cancelable fix: check if event is cancelable before calling preventDefault Dec 12, 2022
@mattlewis92 mattlewis92 merged commit 10bfc51 into mattlewis92:main Dec 12, 2022
@mattlewis92
Copy link
Owner

Released as 7.0.1, thank you!

@danhobbs75
Copy link
Contributor Author

Thanks Matt. Of course, I didn't think to look in the handle directive where there's another one for the mouseDown event. D'oh! I'll do another PR...I'm getting good at this now. :-D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants