Make WordPress Core

Opened 9 years ago

Closed 9 years ago

Last modified 6 years ago

#31412 closed enhancement (fixed)

TinyMCE views improvements

Reported by: iseulde's profile iseulde Owned by: azaozz's profile azaozz
Milestone: 4.2 Priority: normal
Severity: normal Version:
Component: TinyMCE Keywords: has-patch
Focuses: javascript Cc:

Description (last modified by iseulde)

I'm going to use this ticket to propose some improvements to the mce views API, instead of adding this to #29841.

  • Give views the option to display a loader or not. See #29841. We can mark the paragraph and replace it later based on this option. To the user it will look as if nothing happened until there is a match. This is very useful for oEmbed.
  • Remove the wrap and replace options from setContent.
  • Better edit function. Now it's part of the instance, so no need to look at the node for the encoded text. You can call the function provided to refresh the view, no need to touch the DOM any more. You can see the edit functions of the core views are a lot simpler.
  • The gallery and av views now reuse the same edit function.
  • Simpler registration.
  • setContent will call setIframes itself if needed.
  • Selecting another view will pause all players. We could also limit this to the same view type or a set of view types. This allows us to remove a lot of code.
  • Better documentation (though it could be better still).
  • Move the main API to the top of the file.
  • Wrap the view registration in a separate IIFE. Ideally this should go in a separate file.
  • The render method is easier to override.
  • Better detection of already rendered views, so we don't unnecessarily refresh views.
  • I added a proper setLoader method, just like setError.
  • All options set in toView are now added as properties of the instance.

I will do some more testing.
Suggestion are welcome.

Attachments (26)

31412.patch (41.9 KB) - added by iseulde 9 years ago.
31412.2.patch (43.5 KB) - added by iseulde 9 years ago.
31412.3.patch (42.7 KB) - added by iseulde 9 years ago.
31412.4.patch (43.3 KB) - added by iseulde 9 years ago.
31412.5.patch (43.8 KB) - added by iseulde 9 years ago.
31412.6.patch (44.2 KB) - added by iseulde 9 years ago.
31412.7.patch (2.0 KB) - added by iseulde 9 years ago.
31412.8.patch (2.3 KB) - added by iseulde 9 years ago.
31412.9.patch (4.0 KB) - added by iseulde 9 years ago.
31412.10.patch (1.7 KB) - added by iseulde 9 years ago.
31412.11.patch (2.6 KB) - added by iseulde 9 years ago.
31412.12.patch (4.0 KB) - added by iseulde 9 years ago.
31412.13.patch (4.0 KB) - added by iseulde 9 years ago.
31412.14.patch (5.5 KB) - added by iseulde 9 years ago.
31412.15.patch (6.1 KB) - added by iseulde 9 years ago.
31412.16.patch (6.6 KB) - added by iseulde 9 years ago.
31412.17.patch (6.7 KB) - added by iseulde 9 years ago.
31412.18.patch (7.6 KB) - added by iseulde 9 years ago.
31412.19.patch (7.9 KB) - added by iseulde 9 years ago.
31412.20.patch (919 bytes) - added by iseulde 9 years ago.
31412.21.patch (1.1 KB) - added by iseulde 9 years ago.
31412.22.patch (1.7 KB) - added by iseulde 9 years ago.
31412.23.patch (462 bytes) - added by iseulde 9 years ago.
31412.24.patch (2.3 KB) - added by iseulde 9 years ago.
31412-duplicate-iframe-content.diff (797 bytes) - added by mattheu 9 years ago.
Fix duplicate content in setIframes
31412.25.patch (2.9 KB) - added by iseulde 9 years ago.

Download all attachments as: .zip

Change History (55)

@iseulde
9 years ago

#1 @iseulde
9 years ago

edit: function( node ) {
	var gallery = wp.media.gallery,
		self = this,
		frame, data;

	data = window.decodeURIComponent( $( node ).attr('data-wpview-text') );
	frame = gallery.edit( data );

	frame.state('gallery-edit').on( 'update', function( selection ) {
		var shortcode = gallery.shortcode( selection ).string();
		$( node ).attr( 'data-wpview-text', window.encodeURIComponent( shortcode ) );
		wp.mce.views.refreshView( self, shortcode, true );
	});

	frame.on( 'close', function() {
		frame.detach();
	});
}

becomes

edit: function( text, update ) {
	var gallery = wp.media.gallery,
		frame = gallery.edit( text );

	frame.state( 'gallery-edit' ).on( 'update', function( selection ) {
		update( gallery.shortcode( selection ).string() );
	} );

	frame.on( 'close', function() {
		frame.detach();
	} );
}
Last edited 9 years ago by iseulde (previous) (diff)

#2 @iseulde
9 years ago

  • Description modified (diff)

@iseulde
9 years ago

#3 @iseulde
9 years ago

Improved the docs a bit.

@iseulde
9 years ago

@iseulde
9 years ago

@iseulde
9 years ago

@iseulde
9 years ago

#4 @azaozz
9 years ago

In 31546:

TinyMCE: wpView improvements:

  • Better structure, simpler "view" registration, better extensibility.
  • Better inline documentation.
  • Don't show a placeholder for pasted link until we know the link is "embeddable'.

And many more improvements. Props iseulde. See #31412.

@iseulde
9 years ago

@iseulde
9 years ago

@iseulde
9 years ago

#5 @azaozz
9 years ago

In 31548:

TinyMCE: wpView improvements: remove the (obsolete) get/setViewText methods. Update stopping/pausing of multiple ME media players. Props iseulde. See #31412.

@iseulde
9 years ago

@iseulde
9 years ago

@iseulde
9 years ago

@iseulde
9 years ago

@iseulde
9 years ago

#6 @iseulde
9 years ago

  • Description modified (diff)

@iseulde
9 years ago

@iseulde
9 years ago

#7 @iseulde
9 years ago

Hm, there seems to be a tiny memory leak since 4.0 [29615]. editor.on( 'wp-body-class-change', function() {....

@iseulde
9 years ago

@iseulde
9 years ago

@iseulde
9 years ago

#8 @azaozz
9 years ago

In 31559:

TinyMCE: wpView improvements: introduce getText() and remove() methods, improved getInstance(), better docs. Props iseulde. See #31412.

@iseulde
9 years ago

#9 @azaozz
9 years ago

In 31586:

TinyMCE wpView: update the "update" method. Props iseulde. See #31412.

@iseulde
9 years ago

#10 @azaozz
9 years ago

In 31612:

TinyMCE wpView: decode HTML entities before trying to insert view markers. Props iseulde. See #31412.

#11 @azaozz
9 years ago

In 31621:

TinyMCE wpView: revert decoding of HTML entities. Doesn't work in old IE and needs to be more selective. Keep the change from .html() to .text() when getting the content of a node. See #31412.

@iseulde
9 years ago

#12 follow-ups: @iseulde
9 years ago

Left here:

  • Above patch (there's a missing check for MutationObserver).
  • Encoding issue.
  • Media views should pause when switching to the text editor. Previously we did this whenever we could through mejs, but that didn't pause any embeds. I think the best solution is to remove the views when switching (which will destroy any iframes and therefore stop anything that's playing). This is okay since all the views need to be rerendered anyway when switching back to visual (all content is replaced by TinyMCE).

#13 @azaozz
9 years ago

In 31667:

TinyMCE wpView: improve unbinding of mutationObserver in nested iframes. Props iseulde. See #31412.

#14 in reply to: ↑ 12 @azaozz
9 years ago

Replying to iseulde:

Left here:

  • Encoding issue.

As far as I see we will have to do that by hand. It seems it will be better to encode the wpView "text" rather than decode the whole content. If we touch the content we risk breaking double encoding, for example code with && in <pre> tags.

  • Media views should pause when switching to the text editor. Previously we did this whenever we could through mejs, but that didn't pause any embeds. I think the best solution is to remove the views when switching (which will destroy any iframes and therefore stop anything that's playing). This is okay since all the views need to be rerendered anyway when switching back to visual (all content is replaced by TinyMCE).

Right. The whole content of TinyMCE is reloaded on every switch Text => Visual as we are expecting changes. There is no point in keeping the old "views".

Last edited 9 years ago by azaozz (previous) (diff)

@iseulde
9 years ago

@iseulde
9 years ago

#16 @azaozz
9 years ago

In 31689:

TinyMCE wpView: decode HTML entities before trying to match the wpView text string. Props iseulde. See #31412.

#17 @dtbaker
9 years ago

Subscribing. I'll update this TinyMCE view/shortcode example once 4.2 RC is out.

https://github.com/dtbaker/wordpress-mce-view-and-shortcode-editor

(I'm using similar code to this in lots of themes so will have to do a big update)

#18 @mattheu
9 years ago

I've found a small - somewhat edge case issue with setIframes. Since this ticket tackles quite a few things - I'm going to post it here.

If you call setIframes twice in quick succession, then you can end up with the iFrame content being shown twice. This only happens for previews that are sandboxed in an iFrame.

You can replicate this by replacing render() in the initialize method of the av view with render( true ); render( true );

I think the reason for this is the timeout in the setIframes method. content.innerHTML is reset outside of the callback. Calling it twice in quick succession causes the content to be appended to the content from the first call.

A simple fix is to move content.innerHTML = ''; inside the timeout callback - resetting the content innerHTML each time.

Uploading a patch.

#19 @iseulde
9 years ago

@mattheu Sounds good. :)

@mattheu
9 years ago

Fix duplicate content in setIframes

@iseulde
9 years ago

#20 @iseulde
9 years ago

Looking good. :) Probably best to move the callback in the timeout too.

This ticket was mentioned in Slack in #core by johnbillion. View the logs.


9 years ago

#22 in reply to: ↑ 12 @iseulde
9 years ago

Left here: patch above from @mattheu and third point in comment above.
We could move that last one to a new ticket.

This ticket was mentioned in Slack in #core-editor by iseulde. View the logs.


9 years ago

#24 @azaozz
9 years ago

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

In 31740:

TinyMCE: improve setting of the sandboxing iframe inside a view.
Props mattheu, iseulde. Fixes #31412.

This ticket was mentioned in Slack in #core by paulschreiber. View the logs.


9 years ago

This ticket was mentioned in Slack in #core-editor by paulschreiber. View the logs.


9 years ago

#27 @dtbaker
8 years ago

Hey all,

Is there a way to override the default gallery template?

Here is a before/after 4.2 example that I'm struggling with. Surely there's an easier way?

http://stackoverflow.com/questions/31552084/wp-mce-views-gallery-override-default-editor-gallery-template

Thanks!

This ticket was mentioned in Slack in #core-editor by andreiglingeanu. View the logs.


7 years ago

#29 @antpb
6 years ago

#31881 was marked as a duplicate.

Note: See TracTickets for help on using tickets.