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

IText additions/fixes #945

Closed
22 of 23 tasks
kangax opened this issue Nov 2, 2013 · 60 comments
Closed
22 of 23 tasks

IText additions/fixes #945

kangax opened this issue Nov 2, 2013 · 60 comments
Assignees
Labels
Milestone

Comments

@kangax
Copy link
Member

kangax commented Nov 2, 2013

  • Double click should only select text when clicked on one spot
  • Performance suffers when text contains 100+ chars
  • Cursor doesn't appear until left/right/up/down key is pressed
  • Ctrl/Cmd + x
  • Cursor should be positioned properly when clicking on flipped text
  • Fix up/down cursor movement in center/right -aligned & styled text
  • Text selection with shift+keyDown works, but not deselection with shift+keyUp
  • Cursor should be positioned properly when clicking on rotated text
  • Parts of text can be selected with mouse/touch
  • Forward-delete should delete selection (if present)
  • Cursor should stay same size when scaling text
  • Mouseup outside of text after mouse selection should finalize selection
  • Click -> shift+click should create selection
  • Triple click to select a line
  • Change styles correctly when replacing multi-line text
  • Add enter-editing / exit-editing events
  • Fix exit editing when clicking on 2nd text
  • Unit tests
  • Fix styles when copy/pasting
  • Fix line-through
  • Fix char width detection with custom font
  • SVG export (~75% complete ATM)

Should we also fix these?

  • Cursor movements in flipped text are all backwards
@ghost ghost assigned kangax Nov 2, 2013
@ctzurcanu
Copy link

ok.. I was not able to see your tickets..
issues:
-a Mac does not have "backspace" key all things related to that key will not function
-most of the time the cursor is not observable. user should know where the cursor is
-maybe there should be 2 separate events of the sort "on text change complete" and "on changing text"
-maybe consider ability to styling lines with bullets
-any shortcuts for changing text alignment? :D

@ctzurcanu
Copy link

I was pleasantly surprised: iText brings the keyboard up on mobiles. and it functions well enough overall. Thank you!

@kangax
Copy link
Member Author

kangax commented Nov 3, 2013

-a Mac does not have "backspace" key all things related to that key will not function

not sure what you mean. I made "delete" on mac work, and there's also support for "forward delete" on windows

-most of the time the cursor is not observable. user should know where the cursor is

yeah, that's a known bug — see last one (regression actually)

-maybe there should be 2 separate events of the sort "on text change complete" and "on changing text"

there's "text:changed" event fired when text changes

-maybe consider ability to styling lines with bullets

hm, interesting idea but not something i can get to soon. lots of other things first :)

-any shortcuts for changing text alignment? 💃

of course, textAlign should still work

@kangax
Copy link
Member Author

kangax commented Nov 3, 2013

I was pleasantly surprised: iText brings the keyboard up on mobiles. and it functions well enough overall. Thank you!

Glad you noticed. I specifically worked on that (using hidden textarea "hack") :)

@kangax
Copy link
Member Author

kangax commented Nov 3, 2013

Where are you using Fabric, btw? If you don't mind me asking.

@ctzurcanu
Copy link

Fabric will be the platform we will use. IText decided that. I meant keyboard shortcuts for changing text align :D (like a key combination)

@ctzurcanu
Copy link

and yes: you will get invited to a demo of Fabric on what we develop. ASAP

@kangax
Copy link
Member Author

kangax commented Nov 3, 2013

You'll always be able to hook up your own events and functionality. I just wanted to cover some basic functionality like line/word/test jumps, copy/paste, etc. Perhaps we'll move this into a separate module in the future and extend with even more functionality.

Speaking of copy/paste, I've been meaning to look into adding support for copy/paste from user buffer (not just text on canvas). We'll see how it goes...

@ctzurcanu
Copy link

copy/paste from buffer is a very good idea.. i wish you success. i hope to invite you this week.

This was referenced Nov 4, 2013
@wibbetson
Copy link

Hi there, great work on IText. Works great! One things I was wondering however if it is currently possible to copy and paste text from a word document using IText?

@somecodemonkey
Copy link
Contributor

I noticed the keyboard does not prompt from some mobile/touch devices. Specifically my android phone, the keyboard does not display when i attempt to edit text when using the kitchen sink demo.

@kangax
Copy link
Member Author

kangax commented Nov 12, 2013

@wibbetson No way to access user buffer cross-browser without Flash. You can always do it on app level.

@darbs It should work in latest version. Please check again.

@MikeFarrow
Copy link

Could the keyboard appearing be a settable option? It can be disruptive in some full screen apps.

@kangax
Copy link
Member Author

kangax commented Nov 13, 2013

@MikeFarrow for now you can just remove (or set to smth falsey) hiddenTextarea property. But I suppose we can add an option like that.

@somecodemonkey
Copy link
Contributor

The keyboard appears when the textarea is previously selected. I was referring to having the keyboard appear regardless if the textarea is previously selected.

@kangax
Copy link
Member Author

kangax commented Nov 13, 2013

@darbs Not sure I understand. Previously selected how? Keyboard appears when entering text editing and disappears when exiting it.

@somecodemonkey
Copy link
Contributor

@kangax On my galaxy S3 (and any android emulator ive used) when i go to the kitchensink and first click on the itext object in the canvas the keyboard doesnt pop up. The only time the keyboard pops up is when the textarea is clicked or if while the text area is focused, i close the keyboard and click the itext object again.

@kangax
Copy link
Member Author

kangax commented Nov 13, 2013

@darbs what about http://fabricjs.com/test/misc/itext.html? on kitchensink there's extra textarea that might be getting in a way

@somecodemonkey
Copy link
Contributor

@kangax Still no dice, but i noticed that when i click a itext obj the cursor appears then disappears and highlights a word.

@somecodemonkey
Copy link
Contributor

ctrl anything doesnt work.

changing

if (!this.isEditing || e.ctrlKey) return;

to

if (!this.isEditing) return;

fixes since ctrl is still pressed while pressing x,c,a

@somecodemonkey
Copy link
Contributor

IText object doesnt exit Edit mode when another object is selected while in edit mode

@somecodemonkey
Copy link
Contributor

keypress not fired on mobile browsers (Chrome, FF)

@kangax
Copy link
Member Author

kangax commented Nov 16, 2013

Moved SVG import for later (#987)

@somecodemonkey
Copy link
Contributor

Would it be possible to incorporate an event for an itext object such as 'text:enterEdit" or "text:exitEdit" so one could handle certain ui events when a itext object enters edit mode.

@kangax
Copy link
Member Author

kangax commented Nov 18, 2013

Just realized that I completely missed a pretty essential feature of creating selection with click followed by shift click. We do need this, right?

/cc @Kienz @darbs @MikeFarrow @wibbetson @ctzurcanu

@somecodemonkey
Copy link
Contributor

Nice this itext is coming out nicely. Im trying to get the itext object to update from mobile, but the lack of a keypress event is troublesome. I got it semi-working via hiddenTextarea 'input' and itext 'text:changed' listeners but its less then adequate. Any ideas on this?

@kangax
Copy link
Member Author

kangax commented Nov 18, 2013

Can you explain more? Last time I checked, I was able to edit text pretty well on iPad.

@somecodemonkey
Copy link
Contributor

Keypress event doesnt fire on some mobile(chrome, FF), keydown does but with a undefined which value, resulting in the itext object not being updated. After some googling i came accross https://code.google.com/p/chromium/issues/detail?id=118639 , this may be a chrome non IOS specific issue.

@kangax
Copy link
Member Author

kangax commented Nov 20, 2013

Would it be possible to incorporate an event for an itext object such as 'text:enterEdit" or "text:exitEdit" so one could handle certain ui events when a itext object enters edit mode.

Sounds like a good idea actually. I'll add them.

@somecodemonkey
Copy link
Contributor

Awesome thanks, saves me some fabric overrides.

@somecodemonkey
Copy link
Contributor

Cursor in editing mode renders incorrectly spaced for some fonts, particularly 'Headland One' not sure if its itext issue or something else. http://jsfiddle.net/NPMZd/1/

@somecodemonkey
Copy link
Contributor

Itext objects dont exit edit mode when clicking another itext object. http://jsfiddle.net/3LHJG/1/

@kangax
Copy link
Member Author

kangax commented Nov 20, 2013

@Kienz I added "toSVG" links on our test page — http://fabricjs.com/test/misc/itext.html — so that it's easy to see where SVG output is off. The problem is mainly with background; often times it looks perfect (e.g. "Breaking Bad") but sometimes it's off. Haven't figured out yet why this happens.

@ctzurcanu
Copy link

"Itext objects dont exit edit mode when clicking another itext object. "

I've also seen this.

@kangax
Copy link
Member Author

kangax commented Nov 21, 2013

@ctzurcanu @darbs should be fixed now

@kangax
Copy link
Member Author

kangax commented Nov 21, 2013

Cursor in editing mode renders incorrectly spaced for some fonts, particularly 'Headland One' not sure if its itext issue or something else. http://jsfiddle.net/NPMZd/1/

Hm, true. I see it in our tests as well — http://fabricjs.com/test/misc/itext.html (5th example)

@kangax
Copy link
Member Author

kangax commented Nov 21, 2013

I think it's time to fix text line-height in Fabric, which behaves differently than most of the other text editors (google docs on the left, fabric on the right):

screenshot 2013-11-18 13 54 02

I'm not talking about background filling entire line height (some editors do this, some don't); I'm talking about text being on the bottom of line-height box.

I think we should fix this. What do ya all think?

@somecodemonkey
Copy link
Contributor

@kangax with regards to the cursor, i havent had much time to look into this issue but i noticed if you change the font size then the cursor works correctly. eg. canvases[5].getObjects()[0].fontSize = 41 in the console on http://fabricjs.com/test/misc/itext.html .

@kangax
Copy link
Member Author

kangax commented Nov 26, 2013

@darbs as I suspected, this had to do with caching; char widths were being cached before font was loaded. Changing fontSize to 41 was making it work due to cache invalidation (or rather, being unable to use old cache values — those for fontSize=40). This can be taken care of on an application level — making sure font is loaded before rendering text. I also added "caching" boolean to fabric.IText to be able to toggle this behavior.

@kangax
Copy link
Member Author

kangax commented Nov 27, 2013

Moving the remaining of SVG export feature to #987

@kangax kangax closed this as completed Nov 27, 2013
@kdavie
Copy link

kdavie commented Feb 5, 2014

Clicking on an itext object within a group causes an exception (using Chrome 32.0.1700.77, not sure about other browsers). The issue appears to be related to the hidden text area not being initialized properly. As a temporary work around I'm lazy loading the hiddenText from the click handler using the initHiddenTextarea method. It would be great if this resulted in the text being edible in the group, but sadly it does not. Not sure if the intended behavior is for the text to be edible in a group or not, but that is my desired behavior. Perhaps its just my crappy work-around preventing it from being edible? Either way, it shouldn't cause an exception when clicked.

Edit
Using enterEditing instead of initHiddenTextarea gets me a step closer to editable text within a group, but super buggy still (text updates, but not until blur is called & can't see new text until then either). I'll keep poking to see if I can figure out how to properly initialize it for my work-around.

Edit 2
It turns out to be a more widespread bug than I originally thought, has nothing to do with groups. If you add an itext to a canvas and click anywhere outside of the itext, you'll see the error. Having it in a group just helped expose the issue. The code to duplicate is dead simple:

var canvas = new fabric.Canvas('canvas');
var t1 = new fabric.IText('This is a test',{top: 25, fontSize: 12 });
canvas.add(t1);

@GordoRank, thanks for looking into it :)

@GordoRank
Copy link
Contributor

@kdavie I'll take a look at this for @kangax soon as I'm very familiar with the IText code now and should be able to get a PR in very quickly... I've been much busier with work this week than I expected so my contribution to fabric has been minimal. Should be back to normal next week though :)

Should be pretty easy to fix though...

@timo22345
Copy link

Ctrl+End, Ctrl+Home, Ctrl+Shift+Home, Ctrl+Shift+End don't work (in Chrome Windows7). Why reinventing the wheel? HTML textarea has best editing capabilities ready, all key combinations and mouse events and touchevents work out of the box. Why not to use it above canvas? It can be shown when doubleclicked and hidden when editing is ended. Rotations and scalings can be implemented using css.

SVG also lacks support for proper editable multiline textarea. I'm ending up using textarea (or contenteditable p) above svg document and when editing is ended, rendering the text in SVG. The principle is here: http://jsfiddle.net/yRH9Y/.

@GordoRank
Copy link
Contributor

@timo22345

Because Fabric is a library for the manipulation of objects on a canvas.

The visual inconsistencies using the approach you suggest are extremely jarring. It is impossible to match the positioning, layout and styles of the text in an overlayed textarea with what is actually drawn on the canvas, at least not in a precise fashion.

More importantly, using a textarea does not give the kind of control required. You cannot add per character styles like text outline color, text background colors, shadows and so on.

To do so requires using a contenteditable div and separating the individually styled parts of the text into their own elements within this div. When you do this, you find that the differences between what is rendered via html and canvas are even further apart.

Believe me, I spent nearly a month working on it using exactly the method you described. For the purpose of this library, it just doesn't work.

In time all key combinations and events will be supported, but regardless, in order to get the quality and accuracy required, the approach @kangax has taken is absolutely the correct one.

@somecodemonkey
Copy link
Contributor

@GordoRank +1 this.Textarea a method does not look nice, especially with some fonts. It just looks cheap.

@timo22345
Copy link

Good reasons. I still prefer textarea. It works out of the box the same way as I have used to edit text in various text-editing programs: Word, Notepad, html textfields. I'm making a text editor, which a translator appends translated text on comic strips and there it is essential that editing is fast and works exactly like editing works elsewhere (undo redo etc.). So is it possible to make an option to use only textarea instead of current way? The current way looks well nice, but for us user experience is much more essential than good lookingness. And the final rendered result is still as good looking.

@dbhatt900
Copy link

IText is not working in mobile as i am using phonegap for mobile app.

@tad-lispy
Copy link

Multiple lines of text get cramped together when exported to SVG. You can see it in tests provided here: http://fabricjs.com/test/svg_export/text.html. Below is the screenshot.

Multiline IText cramped in SVG

Also, when new line is inserted and then removed, the position in exported SVG is wrong.

Is there a way to disable newlines while this is being fixed?

@kangax
Copy link
Member Author

kangax commented Oct 29, 2014

@lzrski I don't see it happening in that demo (which is fabric.Text, btw, not fabric.IText)

@tad-lispy
Copy link

I still see this error. I use Chromium Version 37.0.2062.120 Ubuntu 14.04 (281580) (64-bit) and Firefox 33.0 on the same machine. This effect is visible only in SVG. Bitmaps exported with createPNGStream, toDataURL or toDataURLWithMultiplier are fine.

@asturur
Copy link
Member

asturur commented Oct 29, 2014

@lzrski This errore is because the page search for cufon.js. Kienz spotted it.
Please open the link with http://fabricjs.com/test/svg_export/text.html#native to get correct behavour.
Some pixel is still misaligned but the fix is in some PR between the push and build distribution.

@tad-lispy
Copy link

I confirm it displays ok at your URL. Thanks @asturur

I have the same error in my app (actually I switched to PNG because of that). Could you please elaborate on the solution or point me to some docs / discussion?

@asturur
Copy link
Member

asturur commented Oct 29, 2014

If you are rendering throught standard behavour ( useNative: true ) it should be ok.
Can you point to your application in someway?

@tad-lispy
Copy link

Thanks @asturur .

As for my app I no longer use SVG (PNG suits me well enough) and the code isn't publicly hosted so I can't point you to the version with errors. I very much appreciate your readiness to take a look though.

In short I was using toSVG method of canvas to store the SVG string in a Backbone.js model. In another view I was just injecting the SVG into a templete being rendered. Everything worked fine except the multiline text.

For convenience of other readers here are docs for IText.useNative flag and a snippet used at the page provided by @asturur :

function addText(text, options) {
    var text = new fabric.Text(text, options);
    text.useNative = document.location.hash === '#native';
    __canvas.add(text);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests