[JBPM-5544] Added basic text wrapping

[Christopher-Chianelli]

Added basic text wrapping support for Text objects. By default, text will not wrap, and will behave as before. To enable text wrapping, call setWrapBoundaries(BoundingBox box,Layout layout), where box is the bounding box for the object where the text resides, and layout is the the text layout within the BoundingBox. This patch was made for the 2.0.287-RELEASE (since that the one KIE Workbench (https://github.com/kiegroup/kie-wb-common) uses). Feedback appreciated.

[romartin]

Hey @Christopher-Chianelli

That sound good for me, not sure if @SprocketNYC had some previous idea for this. Just requested a few changes for the concrete LayoutContainer/Layout, which are intended to just reside in wires, so what you think about moving that concrete code from the Text class to some other class in wires? I'm agree on keeping the setWrapBoundaries and this logic in Text, just moving concrete layouts from it.

Thanks!

[Christopher-Chianelli]

I might be able to remove the Layout component entirely and just use getTextAlign, since the code for TOP, CENTER and BOTTOM layout appear to be the same. I'll investigate if that the case when I go back to work tomorrow.

[romartin]

sure thanks @Christopher-Chianelli !

[Christopher-Chianelli]

After some investigations (and learning how to make a wires lienzo project), I managed to fix the line vertical placement and remove Layout from text @romartin.

[romartin]

Hey @Christopher-Chianelli , just did a try to your stuff but still not sure how to make it work. Please, I have added a new test class for your stuff here, can you give it a try and figure out what I'm missing? Not sure if this does not work at 100% yet or I'm missing to do some another call... but if you run that test, will see that the text is not being wrap yet, any ideas? Thanks!

[Christopher-Chianelli]

@romartin do you have spaces in your text? It only does word wrap currently. I could add an enum so the user could choose wrap style (wrap by word, wrap by word but cut if a word span more than a line, wrap by character, no wrap).

[romartin]

@Christopher-Chianelli well tested both Strings - with spaces and with no spaces. Were you able to run the example link I provided? You can reproduce it there, with and without spaces.

[Christopher-Chianelli]

I see what you mean @romartin; the text position isn't getting updated. However, for some reason my demo doesn't have this issue despite having near identical code https://github.com/Christopher-Chianelli/LienzoDemo

[Christopher-Chianelli]

Nvm, found the fix: remove the child, and add it with the layout @romartin.

[Christopher-Chianelli]

(When updating the wrap boundaries)

[romartin]

Hey @Christopher-Chianelli

Yeah tested your proposed solution and it works fine. It sounds good for me! Good job :)

On the other hand, I would say we should try to use another approach rather than adding/removing text instances on each resize event handler, because it sounds quite expensive... Would you mind trying to make this work in wires and without adding/removing the text instance? This way we'll have the complete picture for Stunner. So once merging this we have to be sure that text wrapping works just when using the primitive Text type (as you already has done), and works as well with all wires shapes and the wires resize stuff.

What I would suggest, as this feature is cool and will be hardly used, is to update the current Lienzo Kitchen Sink, for example just updating the "Wires Resize" example, by adding some text that gets wrapped while resizing, even when clicking on the top buttons for changing the layout. This way will give visibility to that feature and we can always go to the KS page and check whatever we need :), and finally use same/similar code in Stunner as for the KS :) WDYT?

Thanks!!!

[Christopher-Chianelli]

Hey @romartin!

I think the issue is the text shape is changing, and wires probably assume all children shapes don't change on resize. I'll see what I can do.

[romartin]

Thanks @Christopher-Chianelli , lemme know if I can help :)

[Christopher-Chianelli]

@romartin, I managed to make it work without removing/readding the child. Needed to modify WiresLayoutContainer to listen to changes to its children BoundingBox's attributes (call refresh() on change). Couldn't get Lienzo Kitchen Sink working; it compiles fine, but exposes no web pages for some reason (despite there being an index.html in war). I tested on your demo.

[romartin]

Hey @Christopher-Chianelli , thanks great job! Let's try to finish it with the example on the KS ,that would be great! Here are my clues for using the KS:

1. Update the repositories configuration in build.gradle and add mavenCentral() - in order to use your local maven repository, which contains your updates. 2.- Run the command gradle war - it generates the war artifact in the path ./build/libs/lienzo-ks.war 3.- Finally just deploy that war in a tomcat7 - the context path is lienzo-ks

Lemme know if that works!

[romartin]

@Christopher-Chianelli just tested and the latest KS works for me

[Christopher-Chianelli]

@romartin It worked! But correction, you add mavenLocal() to use local maven repositories. Unfortunately, I cannot get it to run inside my IDE.

[Christopher-Chianelli]

@romartin Added text wrap demo to WiresResizeViewComponent. https://github.com/Christopher-Chianelli/lienzo-ks/tree/text-wrap-demo. Need to add mavenLocal() for it to compile until changes are merged.

[romartin]

great job @Christopher-Chianelli , will give this a try in a bit and let you know!! BTW I cannot get KS working from the IDE too. Anyway the idea is that KS is gong to be replaced soon, so do not worry too much for now... Thanks! :)

[manstis]

@romartin @Christopher-Chianelli Looks good to me!

[SprocketNYC]

@Christopher-Chianelli @romartin Is it necessary for Text wrap boundaries to be a BoundingBox ??? You only ever use width and set the width attribute, just setting width would be easier

comments????

[romartin]

yeah @SprocketNYC good point, probably not, not really sure, will let @Christopher-Chianelli answer to this :)

[SprocketNYC]

@Christopher-Chianelli @romartin That way it'll be a real "property" and JSON serialized. it's kind of straddling a limbo zone between state and property

[romartin]

oh sure, good point!! absolutely agree @SprocketNYC . well you just merged this, up to you! I would vote for refactoring this using just a couple of double attributes too :) @Christopher-Chianelli WDYT? Thanks

[Christopher-Chianelli]

Good point @SprocketNYC! However, I can imagine adding a feature where height is used - auto-sizing text to fit box - but that is a feature not in this pull request. I used BoundingBox since the majority of lienzo shapes provides a getBoundingBox method, not a getWidth method. Perhaps we can make Text use doubles internally, but allow you to set them either with setWrapBoundaries(double) or setWrapBoundaries(BoundingBox). WDYT?