feat(IText): deprecate `KeyboardEvent#keyCode` in favor of `key` #7865

ShaMan123 · 2022-04-05T07:13:39Z

deprecate keyCode (mdn states it is) in favor of key | code
support passing functions to keyMap
fix: modifier functions prior to non modifier in calling logic

https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/keyCode https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/key

github-actions · 2022-04-05T07:15:39Z

Code Coverage Summary

> fabric@5.1.0 coverage:report
> nyc report --reporter=lcov --reporter=text

-----------|---------|----------|---------|---------|-----------------------------------------------
File       | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                             
-----------|---------|----------|---------|---------|-----------------------------------------------
All files  |   83.07 |    76.06 |   86.29 |   82.79 |                                               
 fabric.js |   83.07 |    76.06 |   86.29 |   82.79 | ...,30140,30214,30225-30290,30413,30512,30748 
-----------|---------|----------|---------|---------|-----------------------------------------------

ShaMan123

nice and simple

ShaMan123 · 2022-04-05T07:14:54Z

src/mixins/itext_key_behavior.mixin.js

+    else if (e.keyCode in keyMap) {
+      func = keyMap[e.keyCode];
+    }
+    else if ((e.key in this.ctrlKeysMapDown) && (e.ctrlKey || e.metaKey)) {


Shouldn't the ctrl maps come before the general key maps?

eventually yes. Probably there was not overlapping?

No overlapping in fabric. I had in mind the dev wanting to override what happens when a ctrl modifier is active.
Current logic doesn't allow.

a045103 fixes modifier priority

github-actions · 2022-04-05T07:19:56Z

Code Coverage Summary

> fabric@5.1.0 coverage:report
> nyc report --reporter=lcov --reporter=text

-----------|---------|----------|---------|---------|-----------------------------------------------
File       | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                             
-----------|---------|----------|---------|---------|-----------------------------------------------
All files  |   83.07 |    76.06 |   86.29 |   82.79 |                                               
 fabric.js |   83.07 |    76.06 |   86.29 |   82.79 | ...,30140,30214,30225-30290,30413,30512,30748 
-----------|---------|----------|---------|---------|-----------------------------------------------

src/mixins/itext_key_behavior.mixin.js

asturur · 2022-04-05T11:29:29Z

keyCode is probably deprecated, important is that key is fully available in all supported browser.
Probably ie11 was the reason we were on keyCode.

Can we verify that? in case we don't need to support keyCode anymore.
Do the values between key and keyCode differ?

ShaMan123 · 2022-04-05T11:57:44Z

They are totally different.
https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/key#browser_compatibility
All browsers support key and MDN officially suggests using it instead of the rest.

ShaMan123 · 2022-04-05T13:27:00Z

Now if I want to remove keyCode from this PR I will get a conflict with #7864 (This is the part I dislike about splitting PRs - but I do undersrand it's a must)

src/mixins/itext_key_behavior.mixin.js

stale · 2022-04-25T00:41:36Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

asturur · 2022-04-29T00:04:11Z

i think that if we want to ~~duplicate~~ deprecate keycode in favor of key, we should change the maps to have the key and we can leave keycode support for 1 major version with a dev warning.

github-actions · 2022-04-29T03:42:11Z

Code Coverage Summary

> fabric@5.1.0 coverage:report
> nyc report --reporter=lcov --reporter=text

-----------|---------|----------|---------|---------|-----------------------------------------------
File       | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                             
-----------|---------|----------|---------|---------|-----------------------------------------------
All files  |   82.94 |    75.26 |   86.32 |   82.66 |                                               
 fabric.js |   82.94 |    75.26 |   86.32 |   82.66 | ...,30483,30557,30568-30633,30756,30855,31091 
-----------|---------|----------|---------|---------|-----------------------------------------------

github-actions · 2022-04-29T04:11:08Z

Code Coverage Summary

> fabric@5.1.0 coverage:report
> nyc report --reporter=lcov --reporter=text

-----------|---------|----------|---------|---------|-----------------------------------------------
File       | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                             
-----------|---------|----------|---------|---------|-----------------------------------------------
All files  |   82.93 |    75.27 |   86.34 |   82.65 |                                               
 fabric.js |   82.93 |    75.27 |   86.34 |   82.65 | ...,30511,30585,30596-30661,30784,30883,31119 
-----------|---------|----------|---------|---------|-----------------------------------------------

github-actions · 2022-04-29T04:31:43Z

Code Coverage Summary

> fabric@5.1.0 coverage:report
> nyc report --reporter=lcov --reporter=text

-----------|---------|----------|---------|---------|-----------------------------------------------
File       | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                             
-----------|---------|----------|---------|---------|-----------------------------------------------
All files  |   82.88 |     75.1 |   86.34 |    82.6 |                                               
 fabric.js |   82.88 |     75.1 |   86.34 |    82.6 | ...,30521,30595,30606-30671,30794,30893,31129 
-----------|---------|----------|---------|---------|-----------------------------------------------

github-actions · 2022-04-29T04:33:19Z

Code Coverage Summary

> fabric@5.1.0 coverage:report
> nyc report --reporter=lcov --reporter=text

-----------|---------|----------|---------|---------|-----------------------------------------------
File       | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                             
-----------|---------|----------|---------|---------|-----------------------------------------------
All files  |   82.89 |    75.13 |   86.34 |   82.61 |                                               
 fabric.js |   82.89 |    75.13 |   86.34 |   82.61 | ...,30521,30595,30606-30671,30794,30893,31129 
-----------|---------|----------|---------|---------|-----------------------------------------------

in case wild languages and keyboards I am not aware of

github-actions · 2022-04-29T04:41:43Z

Code Coverage Summary

> fabric@5.1.0 coverage:report
> nyc report --reporter=lcov --reporter=text

-----------|---------|----------|---------|---------|-----------------------------------------------
File       | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                             
-----------|---------|----------|---------|---------|-----------------------------------------------
All files  |   82.89 |    75.13 |   86.34 |   82.61 |                                               
 fabric.js |   82.89 |    75.13 |   86.34 |   82.61 | ...,30521,30595,30606-30671,30794,30893,31129 
-----------|---------|----------|---------|---------|-----------------------------------------------

github-actions · 2022-04-29T04:50:14Z

Code Coverage Summary

> fabric@5.1.0 coverage:report
> nyc report --reporter=lcov --reporter=text

-----------|---------|----------|---------|---------|-----------------------------------------------
File       | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                             
-----------|---------|----------|---------|---------|-----------------------------------------------
All files  |   82.87 |    75.23 |   86.34 |   82.58 |                                               
 fabric.js |   82.87 |    75.23 |   86.34 |   82.58 | ...,30525,30599,30610-30675,30798,30897,31133 
-----------|---------|----------|---------|---------|-----------------------------------------------

github-actions · 2022-04-29T05:00:37Z

Code Coverage Summary

> fabric@5.1.0 coverage:report
> nyc report --reporter=lcov --reporter=text

-----------|---------|----------|---------|---------|-----------------------------------------------
File       | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                             
-----------|---------|----------|---------|---------|-----------------------------------------------
All files  |   83.03 |    75.53 |   86.35 |   82.74 |                                               
 fabric.js |   83.03 |    75.53 |   86.35 |   82.74 | ...,30507,30581,30592-30657,30780,30879,31115 
-----------|---------|----------|---------|---------|-----------------------------------------------

github-actions · 2022-04-29T05:01:07Z

Code Coverage Summary

> fabric@5.1.0 coverage:report
> nyc report --reporter=lcov --reporter=text

-----------|---------|----------|---------|---------|-----------------------------------------------
File       | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                             
-----------|---------|----------|---------|---------|-----------------------------------------------
All files  |   83.04 |    75.57 |   86.35 |   82.76 |                                               
 fabric.js |   83.04 |    75.57 |   86.35 |   82.76 | ...,30507,30581,30592-30657,30780,30879,31115 
-----------|---------|----------|---------|---------|-----------------------------------------------

src/mixins/itext_key_behavior.mixin.js

ShaMan123 · 2022-04-29T04:35:18Z

src/mixins/itext_key_behavior.mixin.js

+    else if (e.keyCode in keyMap) {
+      func = keyMap[e.keyCode];
+    }
+    else if ((e.key in this.ctrlKeysMapDown) && (e.ctrlKey || e.metaKey)) {


a045103 fixes modifier priority

src/mixins/itext_key_behavior.mixin.js

github-actions · 2022-05-01T17:04:56Z

Code Coverage Summary

> fabric@5.1.0 coverage:report
> nyc report --reporter=lcov --reporter=text

-----------|---------|----------|---------|---------|-----------------------------------------------
File       | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                             
-----------|---------|----------|---------|---------|-----------------------------------------------
All files  |   83.16 |    76.17 |   85.79 |   82.89 |                                               
 fabric.js |   83.16 |    76.17 |   85.79 |   82.89 | ...,30892,30966,30977-31042,31165,31264,31500 
-----------|---------|----------|---------|---------|-----------------------------------------------

stale · 2022-09-21T04:40:31Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

ShaMan123 added 5 commits April 5, 2022 09:07

deprecate keyCode in favor of key

7886d8c

https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/keyCode https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/key

allow functions as keymap values

d77dd48

Update itext_key_behaviour.js

534400e

Update itext_key_behaviour.js

236a477

JSDOC

f8aebdf

ShaMan123 commented Apr 5, 2022

View reviewed changes

ShaMan123 requested a review from melchiar April 5, 2022 07:43

asturur reviewed Apr 5, 2022

View reviewed changes

src/mixins/itext_key_behavior.mixin.js Show resolved Hide resolved

melchiar reviewed Apr 9, 2022

View reviewed changes

src/mixins/itext_key_behavior.mixin.js Outdated Show resolved Hide resolved

stale bot added the stale Issue marked as stale by the stale bot label Apr 25, 2022

ShaMan123 added text and removed stale Issue marked as stale by the stale bot labels Apr 25, 2022

Merge branch 'master' into deprecate-itext-keycode

c2586ba

ShaMan123 added 2 commits April 29, 2022 07:04

deprecate keyCode

e9d3c4d

warn of deprecation

5e5723b

ShaMan123 added 2 commits April 29, 2022 07:23

deprecate keyCode in favor of code

d1b4e66

better warning

491d6a1

safer approach

0c8b6c2

in case wild languages and keyboards I am not aware of

refactor code

644cb06

ShaMan123 added 2 commits April 29, 2022 07:58

fix(): ctrl keys prior to non modifier keys

a045103

JSDOC

94caabc

ShaMan123 commented Apr 29, 2022

View reviewed changes

Merge branch 'master' into deprecate-itext-keycode

7296c17

stale bot added the stale Issue marked as stale by the stale bot label Sep 21, 2022

ShaMan123 removed the stale Issue marked as stale by the stale bot label Sep 21, 2022

ShaMan123 mentioned this pull request Nov 26, 2022

fix(IText): key maps #8457

Merged

ShaMan123 mentioned this pull request Feb 19, 2023

dep(IText): keyCode and key maps => handleSelectionChange #8709

Open

ShaMan123 closed this Feb 19, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(IText): deprecate `KeyboardEvent#keyCode` in favor of `key` #7865

feat(IText): deprecate `KeyboardEvent#keyCode` in favor of `key` #7865

ShaMan123 commented Apr 5, 2022 •

edited

Loading

github-actions bot commented Apr 5, 2022 •

edited

Loading

ShaMan123 left a comment

ShaMan123 Apr 5, 2022

asturur Apr 5, 2022

ShaMan123 Apr 5, 2022

ShaMan123 Apr 29, 2022 •

edited

Loading

github-actions bot commented Apr 5, 2022 •

edited

Loading

asturur commented Apr 5, 2022

ShaMan123 commented Apr 5, 2022 •

edited

Loading

ShaMan123 commented Apr 5, 2022

stale bot commented Apr 25, 2022

asturur commented Apr 29, 2022 •

edited by ShaMan123

Loading

github-actions bot commented Apr 29, 2022 •

edited

Loading

github-actions bot commented Apr 29, 2022 •

edited

Loading

github-actions bot commented Apr 29, 2022 •

edited

Loading

github-actions bot commented Apr 29, 2022 •

edited

Loading

github-actions bot commented Apr 29, 2022 •

edited

Loading

github-actions bot commented Apr 29, 2022 •

edited

Loading

github-actions bot commented Apr 29, 2022 •

edited

Loading

github-actions bot commented Apr 29, 2022 •

edited

Loading

ShaMan123 Apr 29, 2022 •

edited

Loading

github-actions bot commented May 1, 2022 •

edited

Loading

stale bot commented Sep 21, 2022

feat(IText): deprecate KeyboardEvent#keyCode in favor of key #7865

feat(IText): deprecate KeyboardEvent#keyCode in favor of key #7865

Conversation

ShaMan123 commented Apr 5, 2022 • edited Loading

github-actions bot commented Apr 5, 2022 • edited Loading

Code Coverage Summary

ShaMan123 left a comment

Choose a reason for hiding this comment

ShaMan123 Apr 5, 2022

Choose a reason for hiding this comment

asturur Apr 5, 2022

Choose a reason for hiding this comment

ShaMan123 Apr 5, 2022

Choose a reason for hiding this comment

ShaMan123 Apr 29, 2022 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Apr 5, 2022 • edited Loading

Code Coverage Summary

asturur commented Apr 5, 2022

ShaMan123 commented Apr 5, 2022 • edited Loading

ShaMan123 commented Apr 5, 2022

stale bot commented Apr 25, 2022

asturur commented Apr 29, 2022 • edited by ShaMan123 Loading

github-actions bot commented Apr 29, 2022 • edited Loading

Code Coverage Summary

github-actions bot commented Apr 29, 2022 • edited Loading

Code Coverage Summary

github-actions bot commented Apr 29, 2022 • edited Loading

Code Coverage Summary

github-actions bot commented Apr 29, 2022 • edited Loading

Code Coverage Summary

github-actions bot commented Apr 29, 2022 • edited Loading

Code Coverage Summary

github-actions bot commented Apr 29, 2022 • edited Loading

Code Coverage Summary

github-actions bot commented Apr 29, 2022 • edited Loading

Code Coverage Summary

github-actions bot commented Apr 29, 2022 • edited Loading

Code Coverage Summary

ShaMan123 Apr 29, 2022 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented May 1, 2022 • edited Loading

Code Coverage Summary

stale bot commented Sep 21, 2022

feat(IText): deprecate `KeyboardEvent#keyCode` in favor of `key` #7865

feat(IText): deprecate `KeyboardEvent#keyCode` in favor of `key` #7865

ShaMan123 commented Apr 5, 2022 •

edited

Loading

github-actions bot commented Apr 5, 2022 •

edited

Loading

ShaMan123 Apr 29, 2022 •

edited

Loading

github-actions bot commented Apr 5, 2022 •

edited

Loading

ShaMan123 commented Apr 5, 2022 •

edited

Loading

asturur commented Apr 29, 2022 •

edited by ShaMan123

Loading

github-actions bot commented Apr 29, 2022 •

edited

Loading

github-actions bot commented Apr 29, 2022 •

edited

Loading

github-actions bot commented Apr 29, 2022 •

edited

Loading

github-actions bot commented Apr 29, 2022 •

edited

Loading

github-actions bot commented Apr 29, 2022 •

edited

Loading

github-actions bot commented Apr 29, 2022 •

edited

Loading

github-actions bot commented Apr 29, 2022 •

edited

Loading

github-actions bot commented Apr 29, 2022 •

edited

Loading

ShaMan123 Apr 29, 2022 •

edited

Loading

github-actions bot commented May 1, 2022 •

edited

Loading