WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#30220 closed enhancement (fixed)

Make single responsibility function for outputting Revisions JS templates

Reported by: ericlewis Owned by: wonderboymusic
Milestone: 4.1 Priority: normal
Severity: normal Version: 3.6
Component: Revisions Keywords: has-patch commit
Focuses: javascript Cc:

Description


Attachments (1)

30220.diff (7.7 KB) - added by ericlewis 6 years ago.

Download all attachments as: .zip

Change History (10)

@ericlewis
6 years ago

#1 @ericlewis
6 years ago

  • Milestone changed from Awaiting Review to 4.1

#2 @ericlewis
6 years ago

  • Keywords commit added

#3 @wonderboymusic
6 years ago

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

In 30129:

Add a single responsibility function for outputting Revisions JS templates: wp_print_revision_templates().

Use it in wp-admin/revision.php.

Props ericlewis.
Fixes #30220.

#4 @nacin
6 years ago

Most of wp-admin is really simplistic and self-contained, for better or worse, and keeping the templates inside wp-admin/revision.php was deliberate at the time. As a reviewer I'm not entirely sure why this was done — If this was for a use case specific to something you are building, that's _fine_, but it's worth stating in the bug report.

#5 follow-up: @nacin
6 years ago

If we are going to do something like this, we should probably pass the post object in, rather than relying on global state.

#6 in reply to: ↑ 5 @ocean90
6 years ago

Replying to nacin:

If we are going to do something like this, we should probably pass the post object in, rather than relying on global state.

That. Or use get_post(). Or both. Or pass the post lock state in data so we don't even need the post object?

#7 @SergeyBiryukov
6 years ago

Yep, using the $post global contradicts [21735].

#8 @ericlewis
6 years ago

Sorry for not explaining why this was useful earlier.

While building a post forking plugin, I wanted to use the revision diffing view on an Edit Fork page, so editors can compare changes to the original. This required work in #30221, #30219, #30222, and needs follow-up on #30232.

#9 @nacin
6 years ago

So, I like using $post here, or at least not making it a function argument (as in, $post = get_post() would be fine), because it's only actually used to set a disabled attribute on a button. This should really be set in JS based on the model. It would be lovely to get rid of any kind of state in this function.

Note: See TracTickets for help on using tickets.