WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#28913 closed defect (bug) (fixed)

wpviews: keyboard behaviour for a selected view

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

Description

Current behaviour:

  • Inserting characters doesn't do anything when a view is selected.
  • Pressing backspace deletes the view, but also the "space" it took.
  • Pressing enter adds a new paragraph under the view.
  • Pasting something adds the content to the next paragraph, but sticks it to the existing content.

Behaviour for selected text/image:

  • Inserting characters replaces the selection with the characters.
  • Pressing backspace deletes the selection.
  • Pressing enter also deletes the selection.
  • Pasting something replaces the selection.

Conclusion:

Now that we can place the cursor before and after the view, we should also make the behaviour for a selected view more "natural". Since a view is selected clearly, there's no need to assume the user made a mistake. In 3.9 they couldn't place the cursor before or after a view, so these workarounds had to be made.

We should also fix the behaviour of backspace when the cursor is after the view. At the moment it deletes the view, but also the space it took.

Attachments (6)

28913.patch (2.2 KB) - added by iseulde 7 years ago.
28913.2.patch (2.2 KB) - added by iseulde 7 years ago.
28913.3.patch (12.5 KB) - added by iseulde 7 years ago.
28913.4.patch (2.5 KB) - added by iseulde 7 years ago.
28913.5.patch (4.3 KB) - added by iseulde 7 years ago.
28913.6.patch (4.4 KB) - added by iseulde 7 years ago.

Download all attachments as: .zip

Change History (18)

@iseulde
7 years ago

@iseulde
7 years ago

#1 @iseulde
7 years ago

  • Keywords has-patch added

@iseulde
7 years ago

#2 @azaozz
7 years ago

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

In 29236:

TinyMCE wpView:

  • Typing something replaces a selected view.
  • Pressing backspace deletes the selection.
  • Pressing enter also deletes the selection.
  • Pasting something replaces the selection.
  • Also merge the different 'keydown' handlers.

Props avryl, fixes #28913.

@iseulde
7 years ago

@iseulde
7 years ago

@iseulde
7 years ago

#3 @iseulde
7 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Forgot to handle paste... I'd also be better to replace the other remove functions. :)
Just reopening so we don't forget about this.

#4 @azaozz
7 years ago

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

In 29246:

TinyMCE wpView: better removal of views, replace a selected view when pasting. Props avryl, fixes #28913.

#5 @DrewAPicture
7 years ago

  • Keywords needs-docs added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Could you please add docs for removeView()? A description, @since, and @param at the very least.

#6 follow-up: @iseulde
7 years ago

Currently removeView() is just an internal function.

#7 in reply to: ↑ 6 @DrewAPicture
7 years ago

Replying to avryl:

Currently removeView() is just an internal function.

And I'd like it to have some form of docs :) Docs are as much for core developers as they are for people extending WordPress.

#8 @iseulde
7 years ago

Okay, I didn't know we have to document "private" JS functions. Then the other functions probably also need them.

#9 follow-up: @iseulde
7 years ago

Are there any guidelines for this? I don't see a page in the handbook for JS. Only
http://make.wordpress.org/core/handbook/inline-documentation-standards/php-documentation-standards/

#10 in reply to: ↑ 9 @DrewAPicture
7 years ago

Replying to avryl:

Are there any guidelines for this? I don't see a page in the handbook for JS. Only
http://make.wordpress.org/core/handbook/inline-documentation-standards/php-documentation-standards/

Not yet. I'm writing a post proposing we adopt the JSdoc 3 standard though.

At this point we just need a description of what it does, when it was introduced, and any parameters including the type and description. Something like:

/**
 * What it does.
 * 
 * @since 4.0.0
 *
 * @param {type} view Description.
function removeView( view ) {
	// TODO: trigger an event to run a clean up function.
	// Maybe `jQuery( view ).trigger( 'remove' );`?
	editor.undoManager.transact( function() {
		handleEnter( view );
		editor.dom.remove( view );
	});
}

The hope is to come back later and fix up what's missing (what I'm currently doing primarily for PHP).

Last edited 7 years ago by DrewAPicture (previous) (diff)

#11 @DrewAPicture
7 years ago

  • Keywords needs-docs removed
  • Resolution set to fixed
  • Status changed from reopened to closed

Let's call this fixed and circle back on the docs later.

This ticket was mentioned in IRC in #wordpress-dev by DrewAPicture. View the logs.


7 years ago

Note: See TracTickets for help on using tickets.