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(Textbox): expose methods for overrides + fix resize filckering #7806

Merged
merged 16 commits into from
Mar 25, 2022

Conversation

ShaMan123
Copy link
Contributor

@ShaMan123 ShaMan123 commented Mar 14, 2022

The first 2 methods are to make text more accessible from subclassing

  1. graphemeSplit on instance

  2. wordSplit

  3. fixed line wrapping when word > width
    BEFORE
    ezgif com-gif-maker (14)

FIXED
ezgif com-gif-maker (15)

@github-actions
Copy link
Contributor

github-actions bot commented Mar 14, 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  |   83.23 |    76.64 |    86.4 |   82.96 |                                               
 fabric.js |   83.23 |    76.64 |    86.4 |   82.96 | ...,29877,30002,30082-30147,30270,30369,30595 
-----------|---------|----------|---------|---------|-----------------------------------------------

@github-actions
Copy link
Contributor

github-actions bot commented Mar 14, 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  |   83.19 |     76.6 |   86.41 |   82.92 |                                               
 fabric.js |   83.19 |     76.6 |   86.41 |   82.92 | ...,29877,30002,30082-30147,30270,30369,30602 
-----------|---------|----------|---------|---------|-----------------------------------------------

@ShaMan123 ShaMan123 requested a review from asturur March 14, 2022 19:13
@github-actions
Copy link
Contributor

github-actions bot commented Mar 14, 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  |   83.19 |     76.6 |   86.41 |   82.92 |                                               
 fabric.js |   83.19 |     76.6 |   86.41 |   82.92 | ...,29878,30003,30083-30148,30271,30370,30603 
-----------|---------|----------|---------|---------|-----------------------------------------------

1 similar comment
@github-actions
Copy link
Contributor

github-actions bot commented Mar 14, 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  |   83.19 |     76.6 |   86.41 |   82.92 |                                               
 fabric.js |   83.19 |     76.6 |   86.41 |   82.92 | ...,29878,30003,30083-30148,30271,30370,30603 
-----------|---------|----------|---------|---------|-----------------------------------------------

@ShaMan123 ShaMan123 changed the title chore(Textbox): expose methods for overrides fix(Textbox): expose methods for overrides + fix resize filckering Mar 15, 2022
@ShaMan123 ShaMan123 requested a review from melchiar March 15, 2022 07:29
@asturur
Copy link
Member

asturur commented Mar 21, 2022

This seems good, just give some explanation on why we are not using the offset information on the first pass of measuring.
If we do, then we don't need to remeasure later, we can use cached measures.
Or am i missing the point of that double loop?
(i think i because we want to know immediately which is the large word of all text before we take decisions on intermediate long words

@ShaMan123
Copy link
Contributor Author

ShaMan123 commented Mar 21, 2022

i think i because we want to know immediately which is the large word of all text before we take decisions on intermediate long words

Absolutely!
Did I miss styles or something in the first pass?

@github-actions
Copy link
Contributor

github-actions bot commented Mar 21, 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  |   83.23 |     76.6 |   86.41 |   82.96 |                                               
 fabric.js |   83.23 |     76.6 |   86.41 |   82.96 | ...,29878,30003,30083-30148,30271,30370,30603 
-----------|---------|----------|---------|---------|-----------------------------------------------

@ShaMan123
Copy link
Contributor Author

ShaMan123 commented Mar 21, 2022

I don't think a util should be overridden, that's why I've exposed graphemeSplit
Renamed _measureWord to measureWord and made it public. Crucial for overrides and completes what I was after.
But is kind of BREAKING (the method was private so not so much)

@ShaMan123
Copy link
Contributor Author

ShaMan123 commented Mar 21, 2022

2 tests are failing, no idea why yet FIXED

now PR is non breaking
@ShaMan123
Copy link
Contributor Author

Good on my end.
Ready to merge

@github-actions
Copy link
Contributor

github-actions bot commented Mar 22, 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  |   83.19 |    76.56 |   86.41 |   82.91 |                                               
 fabric.js |   83.19 |    76.56 |   86.41 |   82.91 | ...,29879,30004,30084-30149,30272,30371,30607 
-----------|---------|----------|---------|---------|-----------------------------------------------

@asturur
Copy link
Member

asturur commented Mar 25, 2022

this is a good nice fix and improvement.
And of little scope, thanks.

@asturur asturur merged commit 3be3531 into master Mar 25, 2022
@ShaMan123 ShaMan123 deleted the textbox-overrides branch March 25, 2022 23:11
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.

2 participants