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

Allow cutted words in a Textbox shape #4513

Closed
wants to merge 3 commits into from

Conversation

JSteunou
Copy link
Contributor

@JSteunou JSteunou commented Nov 28, 2017

It's a way to mimic html/css world white-space normal or pre-wrap property, because sometime you want the text to fit exactly in the width you set no matter what.

Example here http://jsfiddle.net/Da7SP/736/ one with minWidth one without, both with the option activated and a last one without it.

this would close #2376

@asturur
Copy link
Member

asturur commented Nov 28, 2017

oh this has an open issue around the tracker.
With a similar implementation that i provided some time ago ( before text rewrite so no more usable ).

This has to wait 2.0 release we can discuss it meantime to see perf issue/testing/ problem that may arise.

@JSteunou
Copy link
Contributor Author

Perf are not bad, it's just like having more tiny words. Like the other PR I will need guidance for testing.

I found these related issue #2376 & #4000

@lsiann
Copy link

lsiann commented Mar 31, 2018

The last letter of a cutted word can not be selected.Maybe need a fix somewhere else

@sentoro
Copy link

sentoro commented Apr 25, 2018

Does anyone going to fix this issue? Text wrap still not working for me in 2.0

@asturur
Copy link
Member

asturur commented Apr 25, 2018

What is to be fixed actually?

@sentoro
Copy link

sentoro commented Apr 26, 2018

Long words wrap is not working. (I meant this issue #2376).
I assume, this pr should fix problem with cutting long words in text box is not?

p.s I've tried code from this pr (with allowCuttedWords option) and wrap for long words is working now, but still reproduce problem with incorrect cursor position. Each new line move cursor to one symbol back.
And last letters of a cutted word can not be selected, as mentioned above.

@blobinabottle
Copy link
Contributor

hey @asturur any idea why the bug mentioned? The graphemeLines returned by _wrapLine seems correct. Where would you look to try to fix it?

I know it's not in your roadmap, but this functionality could help us a lot :-)

@asturur
Copy link
Member

asturur commented Aug 22, 2018

We can revisit it. I do not remember.
I wonder if it makes sense to break works in this way, without a breaking sign in the middle.

@blobinabottle
Copy link
Contributor

The use for us is more about using asian language (without spaces apparently) in textbox and being able to resize them. For the moment it's impossible (because considered as a long non-breaking word).

@asturur
Copy link
Member

asturur commented Aug 22, 2018

Apparently.
Maybe they use other dividers, while fabricjs split just with space, like an illitterate barbarian.
That is something to fix anyway, if we did not do already, including the other 2 or 3 unicode spaces that are out there.

@blobinabottle
Copy link
Contributor

http://www.fileformat.info/info/unicode/category/Zs/list.htm I guess this is an helping list. I see U+3000 as ideographic space

@blobinabottle
Copy link
Contributor

But this seems way more complicated as Japanese doesn't have space at all. There are libraries to detect words:
https://stackoverflow.com/questions/49367783/js-split-for-japanese-string

After trying in several softwares they just consider Japanese as a big long word:
https://cl.ly/e2600c23a296 (result in Illustrator, ideally it should be segmented as two "words" '公開' and '文書' )

So I think the approach of this PR is probably the easiest. It's working whatever the language you're using.

@asturur
Copy link
Member

asturur commented Aug 22, 2018

what we can do is making the splitWords function a part of fabric.util as much as grapheme split so that devs can override it when they need to support other languages.

@asturur
Copy link
Member

asturur commented Aug 22, 2018

Is also still unclear if the wordbreaking should trigger just for words longer than width or always to fill up spaces. I can foresee people coming with good reasons for the opposite feature we will choose.

@blobinabottle
Copy link
Contributor

That doesn't make sense. If you try any other softwares it behave as this PR...
And you can still use the option allowCuttedWords for the ones unhappy about it. No?

@asturur
Copy link
Member

asturur commented Aug 22, 2018

i think you misunderstood me.
Should you split each word that is bigger than width, or split each word to fit width, when the option is active?

@blobinabottle
Copy link
Contributor

Ah but then that's something else; "hyphenation" I think. And I guess you could only make it for English (or languages where you know how to cut words).
But you can only find this options in "advanced" softwares (illustrator, indesign, photoshop). Sketch Figma and Framer are doing as this PR and they have no option about hyphenation.

@blobinabottle
Copy link
Contributor

Google Slides and keynote are doing the same (no hyphenation): breaking word longer than the desired width.

@asturur
Copy link
Member

asturur commented Aug 22, 2018

I also wonder if splitTextIntoLines should be a fabric.util function so that everyone does what he wants after an interface is defined.

I m kind of scared to add behaviours that are prone to interpretation about how they should work.

I can see 3 ways on how break words should work.

split longer word on the same line of other small words.

start a new line with the long word and break the word.

break every word so that there are never white spaces at the end of lines.

I m also unsure if the style calculation needs adjustments. since for now we assume that a line break is always a space or a newline, so we know how to move in the style object.

@blobinabottle
Copy link
Contributor

What I can tell is that this PR is working pretty well with styles.
toSVG is perfect as well.

Only issues I saw:

  • cursor and selection position (we have 1 extra char calculated by line created by cutted word)
  • if you resize big texttblock (font size > 40px) to a veiry small width, cutting word will stop working.

I can offer to beta-test this PR as much as possible (if this can help you :-D)

@asturur
Copy link
Member

asturur commented Aug 22, 2018 via email

@asturur
Copy link
Member

asturur commented Aug 22, 2018

But yes please feel free to debug, clean, and improve this idea.

@blobinabottle
Copy link
Contributor

Do you have an idea where to start to track the cursor bug? The text blocks are quite complex to "dig in" ...
What methods would you instinctively look in priority?

@asturur
Copy link
Member

asturur commented Aug 22, 2018

I do not remember now, i need to read on the computer. I do remember that measuring text in order to split it, instead of after splitting it, was a problem.

I ll be back in the evening and try to point you in some direction

@JSteunou
Copy link
Contributor Author

FYI I'm not a big fan of how I did this. It should have been written as a recursive functional code, but the old code is imperative so I kept imperative code. But for cases like this one, imperative is not easy to read.

@asturur
Copy link
Member

asturur commented Aug 23, 2018

you should propose better code if you have it. readibility can be addressed with comments.

I care it works, i remember my first patch could split a long word in many lines.

@stale
Copy link

stale bot commented Jan 25, 2020

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.

@stale stale bot added the stale Issue marked as stale by the stale bot label Jan 25, 2020
@stale stale bot closed this Feb 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Issue marked as stale by the stale bot
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Entering text in a Textbox object expands it when there aren't newlines
5 participants