WordPress.org

Make WordPress Core

Opened 15 months ago

Closed 11 months ago

Last modified 8 weeks ago

#23263 closed enhancement (fixed)

Move Media's backbone template regexes

Reported by: markjaquith Owned by:
Milestone: 3.6 Priority: normal
Severity: normal Version: 3.5
Component: Template Keywords: has-patch
Focuses: Cc:

Description

Media has custom Backbone templating (because PHP is stupid and sometimes parses <% %> as PHP code — I know, I know). That templating should be available elsewhere instead of being specific to media.

		template: _.memoize( function( id ) {
			var compiled,
				options = {
					evaluate:    /<#([\s\S]+?)#>/g,
					interpolate: /\{\{\{([\s\S]+?)\}\}\}/g,
					escape:      /\{\{([^\}]+?)\}\}(?!\})/g,
					variable:    'data'
				};

			return function( data ) {
				compiled = compiled || _.template( $( '#tmpl-' + id ).html(), null, options );
				return compiled( data );
			};
		})

Attachments (4)

23263.diff (2.5 KB) - added by wonderboymusic 14 months ago.
23263.2.diff (1.9 KB) - added by wonderboymusic 14 months ago.
23263.3.diff (5.8 KB) - added by wonderboymusic 14 months ago.
23263.4.diff (6.8 KB) - added by wonderboymusic 14 months ago.

Download all attachments as: .zip

Change History (19)

comment:1 rmccue15 months ago

Originally discussed in #22344 and implemented in r22415.

Last edited 15 months ago by rmccue (previous) (diff)

wonderboymusic14 months ago

comment:2 follow-up: wonderboymusic14 months ago

  • Keywords has-patch added; needs-patch removed

Seems like this needs to load every time we load Backbone, and it itself requires Underscore, otherwise I couldn't find a great place to load it. Introduces wp.template func in JS

comment:3 in reply to: ↑ 2 rmccue14 months ago

Replying to wonderboymusic:

Seems like this needs to load every time we load Backbone, and it itself requires Underscore, otherwise I couldn't find a great place to load it. Introduces wp.template func in JS

Patch still includes references to revisions.js and wp-jquery-ui-slider.js, otherwise looks good.

wonderboymusic14 months ago

wonderboymusic14 months ago

comment:4 follow-up: wonderboymusic14 months ago

Latest patch gets rid of cruff and removes the template func from media

wonderboymusic14 months ago

comment:5 in reply to: ↑ 4 rmccue14 months ago

Replying to wonderboymusic:

Latest patch gets rid of cruff and removes the template func from media

I think we should just set media.template = wp.template to maintain backwards compatibility here. I know that people are already using media-related JS in the wild, probably including this, so we don't want to break that too much.

comment:6 rmccue13 months ago

Any chance of getting this committed? I just ran into this again and I want to avoid loading the media models on the front end.

comment:7 follow-ups: nacin13 months ago

This was committed in [23506/trunk/wp-includes/js/template.js] — but media went untouched, so that should be updated. I agree with media.template = wp.template.

Also, does anyone else think that wp.template is far too generic for what this is? wp.backboneTemplate? wp.backbone.template? Is there something else that might end up being a part of a wp.backbone object that could justify that?

comment:8 in reply to: ↑ 7 rmccue13 months ago

Replying to nacin:

This was committed in [23506/trunk/wp-includes/js/template.js] — but media went untouched, so that should be updated. I agree with media.template = wp.template.

Aha, missed that, thanks.

Also, does anyone else think that wp.template is far too generic for what this is? wp.backboneTemplate? wp.backbone.template? Is there something else that might end up being a part of a wp.backbone object that could justify that?

The original is actually _.template, so wp._.template or wp.underscore.template seems more appropriate. With that in mind, there's basically no reason to override any other Underscore.js function that I can think of.

With Backbone, I could see a custom Backbone.sync which uses admin-ajax.php instead. At the moment it's doing it at a deeper level (jQuery.ajax is replaced by wp.media.ajax) and each model appears to have its own sync code instead.

comment:9 ryan11 months ago

  • Milestone changed from 3.6 to Future Release

comment:10 in reply to: ↑ 7 nacin11 months ago

  • Milestone changed from Future Release to 3.6

Replying to nacin:

This was committed in [23506/trunk/wp-includes/js/template.js] — but media went untouched, so that should be updated. I agree with media.template = wp.template.

Moving this back to 3.6, this has a commit against it.

#24424 supersedes much of this.

comment:11 nacin11 months ago

Also, I'm fine with wp.template().

comment:12 koopersmith11 months ago

In 24366:

Alias media.template to wp.template. See #23263, #24424.

comment:14 koopersmith11 months ago

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

comment:15 ircbot8 weeks ago

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

Note: See TracTickets for help on using tickets.