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(IText): regain focus on mouse move #8179

Merged
merged 1 commit into from
Aug 25, 2022
Merged

Conversation

ShaMan123
Copy link
Contributor

@ShaMan123 ShaMan123 commented Aug 24, 2022

fixes #8177 caused by #3759 (that was hacky) and fixes #3661 that was the motivation for #3759

This issue was caused because the hiddenTextarea looses focus after mousedown.
So I have added a focus call in mousemove that fixes both issues.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 24, 2022

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.1 |    74.56 |   84.46 |    80.7 |                                               
 fabric.js |    82.1 |    74.56 |   84.46 |    80.7 | ...,27453,27511,27521-27565,27673,27760,27964 
-----------|---------|----------|---------|---------|-----------------------------------------------

@ShaMan123 ShaMan123 added the text label Aug 24, 2022
@ShaMan123 ShaMan123 requested a review from asturur August 24, 2022 19:19
@asturur
Copy link
Member

asturur commented Aug 24, 2022

We need to backport this to 5.x too

@asturur
Copy link
Member

asturur commented Aug 24, 2022

To be clear #3759 was as hacky as this newer fix.
It restored focus on mouseout. this restore focus on mousemove.
I fail to understand how this is more solid that the other.
The old code was losing focus on textinput because hovering on textinput equal to a mouseout from canvas and so it was triggering.
With this new fix hovering over the itext will loose focus anyway from the newly selected text input. or am i wrong?

@@ -398,6 +398,9 @@ import { removeFromArray } from '../util/internals';
return;
Copy link
Contributor Author

@ShaMan123 ShaMan123 Aug 25, 2022

Choose a reason for hiding this comment

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

this is why it is more solid
focusing will trigger only if in editing mode and after mousedown

@ShaMan123 ShaMan123 merged commit cb4dd79 into master Aug 25, 2022
@ShaMan123
Copy link
Contributor Author

ShaMan123 commented Aug 25, 2022

We need to backport this to 5.x too

why are we putting effort into v5?
Done e8704f4

ShaMan123 added a commit that referenced this pull request Aug 25, 2022
backports #8179

fixes #8177 caused by #3759
and fixes #3661 that was the motivation for #3759

This issue was caused because the hiddenTextarea looses focus after mousedown.
So I have added a focus call in mousemove that fixes both issues.
@ShaMan123 ShaMan123 deleted the fix-itext-focus-blur branch September 11, 2022 23:36
frankrousseau pushed a commit to cgwire/fabric.js that referenced this pull request Jan 6, 2023
fixes fabricjs#8177 caused by fabricjs#3759
and fixes fabricjs#3661 that was the motivation for fabricjs#3759

This issue was caused because the hiddenTextarea looses focus after mousedown.
So I have added a focus call in mousemove that fixes both issues.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants