WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#28595 closed defect (bug) (fixed)

wpviews: "catch" the cursor and create a fake cursor so that the "cursor" can be set on either side of the view

Reported by: iseulde Owned by:
Milestone: 4.0 Priority: normal
Severity: normal Version: 3.9
Component: TinyMCE Keywords: has-patch
Focuses: javascript Cc:

Description

Also fixes #28594, #28593 and #28257.

Patch coming. Atm this works perfectly well in Safari and Chrome. It needs some adjustments for Firefox. Haven't tested IE yet, brrr.

So this patch adds a body and two <p> tags before and after it. The body now is contenteditable="false", the parent wrap is not. Now the view can "catch" the cursor. We can detect it and create a fake cursor on the relevant side of the view. Keyboard input can be manipulated for those positions.

The fake cursor is as high as the view and you can't see the difference between the real and the fake one with the naked eye. :)

Moving around in the editor with arrow keys feels a lot more natural now.

Attachments (21)

28595.patch (18.5 KB) - added by iseulde 3 years ago.
Screen Shot 2014-06-19 at 14.54.34.png (131.8 KB) - added by iseulde 3 years ago.
Screen Shot 2014-06-19 at 14.56.20.png (8.6 KB) - added by iseulde 3 years ago.
28595.2.patch (19.3 KB) - added by iseulde 3 years ago.
28595.3.patch (26.2 KB) - added by iseulde 3 years ago.
28595.4.patch (25.7 KB) - added by iseulde 3 years ago.
28595.5.patch (26.0 KB) - added by iseulde 3 years ago.
28595.6.patch (1.3 KB) - added by iseulde 3 years ago.
28595.7.patch (2.7 KB) - added by iseulde 3 years ago.
28595.8.patch (2.7 KB) - added by iseulde 3 years ago.
28595.9.patch (2.6 KB) - added by iseulde 3 years ago.
28595.10.patch (4.2 KB) - added by iseulde 3 years ago.
28595.11.patch (5.1 KB) - added by iseulde 3 years ago.
28595.12.patch (5.5 KB) - added by iseulde 3 years ago.
28595.13.patch (2.5 KB) - added by iseulde 3 years ago.
28595.14.patch (1.2 KB) - added by iseulde 3 years ago.
28595.15.patch (2.2 KB) - added by iseulde 3 years ago.
28595.16.patch (2.2 KB) - added by iseulde 3 years ago.
28595.17.patch (2.3 KB) - added by iseulde 3 years ago.
28595.18.patch (2.4 KB) - added by iseulde 3 years ago.
28595.19.patch (2.1 KB) - added by iseulde 3 years ago.

Download all attachments as: .zip

Change History (60)

@iseulde
3 years ago

#1 @iseulde
3 years ago

  • Keywords has-patch added

#2 @iseulde
3 years ago

Of course it flashes, but it looks like this.

#3 @iseulde
3 years ago

In the next patch, moving the cursor next to a view should also work in Firefox. No more double cursors. This should also fix the resize icon that was visible for less than a second. Let me know if you can still spot it.
Pressing backspace will remove the node before the view if it's empty, instead of moving the cursor there.
If the cursor is anywhere in or next to the view where it shouldn't be, the cursor wil be set properly.
If the editor is loaded and the first thing in it is a view, focussing the editor will set the cursor before the first view.

@iseulde
3 years ago

#4 @iseulde
3 years ago

So at the moment this works perfectly well for me in Safari, Chrome and Firefox.
The only thing left is setting the cursor properly when clicking before or after the view. In Chrome and Safari the cursor will always be placed after the view. Firefox doesn't do anything.

Now I should make this work for the latest IE versions. :(

#5 @iseulde
3 years ago

Of course it only works half in in IE 11. Sigh. Will try to fix it tomorrow.

@iseulde
3 years ago

@iseulde
3 years ago

@iseulde
3 years ago

#6 @azaozz
3 years ago

In 28994:

TinyMCE: improve the way wpViews work. Add two paragraphs and capture the caret in them on clicking before/after/left/right of a view or moving the caret with the arrow keys, then show a "fake" caret.

This makes it much more "natural" to move the caret with the arrow keys and to add paragraphs before a view when it is the first element or after a view when it's last.

Props avryl, see #28595.

#7 @iseulde
3 years ago

  • Milestone changed from Awaiting Review to 4.0

#8 @iseulde
3 years ago

Also related: #28256 and #28258.

#9 @azaozz
3 years ago

In 29004:

Remove debug remnants, see #28595

#10 @iseulde
3 years ago

A couple of issues:

  • When a view is selected, pressing the up or down arrow key should immediately move the cursor to the block above or below the view instead of before or after the view.
  • Selecting some text that touches the view will remove part of it when pressing backspace.
  • When a view is the first block in the editor and the page is loaded, the cursor is correctly set before the view, but it doesn't react to key presses. (The editor doesn't seem to be focussed.)

@iseulde
3 years ago

#11 @iseulde
3 years ago

.6 fixes issue 1.

@iseulde
3 years ago

#12 @iseulde
3 years ago

.7 fixes issues 1 and 2.

@iseulde
3 years ago

#13 @iseulde
3 years ago

Left some whitespace in .7

@iseulde
3 years ago

@iseulde
3 years ago

@iseulde
3 years ago

#14 @iseulde
3 years ago

.11 fixes the three issues above + hides the cursor on blur + fixes the case where the cursor is before or after the view and wrongly moves it.

@iseulde
3 years ago

#15 @azaozz
3 years ago

In 29010:

TinyMCE wpView:

  • When a view is selected, pressing the up or down arrow key should move the caret to the block above or below the view.
  • Selecting some text that touches the view and deleting it should not remove part of the view.
  • Show/hide the "fake" carets on editor focus/blur.
  • Don't create new paragraphs before or after a view on pressing the arrow keys or delete key. Paragraphs are created on pressing Enter.

Props avryl, see #28595.

#16 @iseulde
3 years ago

Another issue: the colour of the cursor doesn't change based on the colour and background of the theme, so if the colour is white-ish, then the native cursor has that colour, but the "fake" cursor stays black. This can be solved by getting the colour of the body when TinyMCE loads (with a small timeout so any CSS has the time to load) and change it again whenever the post format changes.

@iseulde
3 years ago

#17 @iseulde
3 years ago

And another issue... If the body has some padding on the left, clicking on the padding before a view doesn't set the cursor.

#18 @azaozz
3 years ago

In 29022:

TinyMCE wpView:

  • Improve the fake caret hide/show.
  • Improve getView() speed.
  • Add callback for showing the proper path when a view is selected. This currently doesn't work, will start working after we update TinyMCE.

props avryl, see #28567, #28595.

#19 @iseulde
3 years ago

More related tickets: #28255 and #28266.

@iseulde
3 years ago

#20 @azaozz
3 years ago

In 29126:

TinyMCE wpView: fix selecting views on click, part props avryl, see #28595

@iseulde
3 years ago

#21 @azaozz
3 years ago

In 29127:

TinyMCE wpView: better handling of the Enter key, props avryl, see #28595

@iseulde
3 years ago

@iseulde
3 years ago

#22 @iseulde
3 years ago

.17: Cast off commands targeted to a view, except undo, redo and RemoveFormat. Disable the link button when a view is selected.

#23 @azaozz
3 years ago

In 29182:

TinyMCE wpView: remove unused code, props avryl, see #28595

#24 @azaozz
3 years ago

In 29183:

TinyMCE wpView:

  • Cast off commands targeted to a view except undo, redo, RemoveFormat and mceToggleFormat (bold, italic, etc.).
  • Disable the link and unlink buttons when a view is selected.

Props avryl, see #28595

@iseulde
3 years ago

#25 @azaozz
3 years ago

In 29184:

TinyMCE wpView: handle execCommand when the "fake caret" P is selected, props avryl, see #28595

#26 @iseulde
3 years ago

Things left here:

  • We need to colour the cursor based on the body's font colour.
  • If the body has padding on the left or right, clicking just next to the view won't work.

@iseulde
3 years ago

#27 follow-up: @iseulde
3 years ago

Refreshed the earlier patch to colour the cursor.

#28 @iseulde
3 years ago

Another thing... We're going to have to use the mousedown event instead of the click event to place the cursor next to the view. A normal cursor is also set on mousedown. In Safari the cursor is now set after the view on mousedown by the browser, and before the view on click by us. That looks weird. :)

#29 @iseulde
3 years ago

I tried this in iOS...

  • The native cursor never hides. I'm not sure if we can work around that. I think the cursor just always takes some space if the editor is focussed.
  • I can't select a view. No matter how many times I click on it. :)

Other than that it seems to work quite well. It's just hard to set the cursor next the view because there's not much space before or after it. The looks of the cursor is also different. Thick blue vs. thin black.

#30 in reply to: ↑ 27 @azaozz
3 years ago

Replying to avryl:

Refreshed the earlier patch to colour the cursor.

Thinking we should use CSS3 currentcolor instead, works in IE9+.

(iOS Safari) I can't select a view. No matter how many times I click on it. :)

I can but only after I close the keyboard... Seems no 'click' event is fired in iOS Safari in contenteditable if the keyboard is open!?

#31 @azaozz
3 years ago

In 29245:

TinyMCE wpVIew: use CSS3 currentcolor for the fake carets, see #28595.

#32 @iseulde
3 years ago

Do we need to do anything special here for RTL? Or does that automatically resolve somehow by having a different keyboard/computer?

#33 @azaozz
3 years ago

In 29273:

wpView: make sure the editor is focused before selecting/deselecting a view, or IE may throw an invalid range error, see #28595.

#34 @azaozz
3 years ago

In 29298:

TinyMCE wpView:

  • Fix opening the media modal on clicking Edit in Firefox.
  • Fix range errors when restoring the selection bookmark in IE11 after editing a view.

See #28595.

#35 @azaozz
3 years ago

  • Resolution set to fixed
  • Status changed from new to closed

This works well, good job @avryl! New tickets for any bugs please.

#36 follow-up: @azaozz
3 years ago

In 29463:

TinyMCE: update wpview and editimage plugins for 4.1.3. Add show/hide of the Edit and Delete buttons on views and images on 'touchend'. See #28595, #29166

#37 @azaozz
3 years ago

In [29471]:

TinyMCE: fix the 'editimage' plugin for touch devices. Better attempt to hide the onscreen keyboard when the media modal opens and TinyMCE is in focus. See #28595, #29166

#38 in reply to: ↑ 36 @ocean90
3 years ago

Replying to azaozz:

In 29463:

TinyMCE: update wpview and editimage plugins for 4.1.3. Add show/hide of the Edit and Delete buttons on views and images on 'touchend'. See #28595, #29166

This includes an issue for touch screens, see #29235.

#39 @azaozz
3 years ago

In 29536:

TinyMCE wpView: remove CSS transition for the fake caret. Can have very annoying side effect: the whole page shifts a bit. See #28595.

Note: See TracTickets for help on using tickets.