Opened 5 months ago
Closed 3 weeks ago
#23263 closed enhancement (fixed)
Move Media's backbone template regexes
| Reported by: |
|
Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | 3.6 |
| Component: | Template | Version: | 3.5 |
| Severity: | normal | Keywords: | has-patch |
| 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)
Change History (18)
wonderboymusic
— 4 months ago
comment:2
follow-up:
↓ 3
wonderboymusic
— 4 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
rmccue
— 4 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.
wonderboymusic
— 4 months ago
wonderboymusic
— 4 months ago
comment:4
follow-up:
↓ 5
wonderboymusic
— 4 months ago
Latest patch gets rid of cruff and removes the template func from media
wonderboymusic
— 4 months ago
comment:5
in reply to:
↑ 4
rmccue
— 4 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
rmccue
— 3 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:
↓ 8
↓ 10
nacin
— 3 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
rmccue
— 2 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:10
in reply to:
↑ 7
nacin
— 3 weeks 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
nacin
— 3 weeks ago
Also, I'm fine with wp.template().
comment:12
koopersmith
— 3 weeks ago
In 24366:
comment:13
rmccue
— 3 weeks ago
\o/
comment:14
koopersmith
— 3 weeks ago
- Resolution set to fixed
- Status changed from new to closed
Originally discussed in #22344 and implemented in r22415.