Make WordPress Core

Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#24804 closed task (blessed) (fixed)

Tighten up revisions UI metadata

Reported by: markjaquith's profile markjaquith Owned by:
Milestone: 3.6 Priority: normal
Severity: normal Version: 3.6
Component: Revisions Keywords: i18n-change
Focuses: Cc:

Description

A few small things:

  • Pin the slider to the top when you scroll
  • Drop the "To:" in single mode
  • Last spit-shine before this goes out the door

Attachments (23)

24804.pin-to-top.diff (5.2 KB) - added by markjaquith 11 years ago.
24804.plus-loading-indicator-fixes.diff (6.2 KB) - added by markjaquith 11 years ago.
24804.3.diff (7.3 KB) - added by ocean90 11 years ago.
24804.diff (7.5 KB) - added by aaroncampbell 11 years ago.
24804.2.diff (15.9 KB) - added by nacin 11 years ago.
24804.4.diff (15.5 KB) - added by nacin 11 years ago.
Probably missing some RTL
24804.5.diff (16.7 KB) - added by markjaquith 11 years ago.
24804.6.diff (19.3 KB) - added by markjaquith 11 years ago.
24804.7.diff (19.5 KB) - added by markjaquith 11 years ago.
24804.8.diff (18.7 KB) - added by nacin 11 years ago.
24804.9.diff (19.1 KB) - added by nacin 11 years ago.
24804.10.diff (1.3 KB) - added by ocean90 11 years ago.
fixie7.diff (321 bytes) - added by adamsilverstein 11 years ago.
don't pin controls when scrolling in ie7
24804.11.diff (10.2 KB) - added by nacin 11 years ago.
24804.12.diff (15.0 KB) - added by nacin 11 years ago.
24804.13.diff (16.9 KB) - added by markjaquith 11 years ago.
More responsive slider. Less crazy with few revisions.
24804.14.diff (503 bytes) - added by DrewAPicture 11 years ago.
(Browsing requires JavaScript) for no-js
24804.15.diff (5.7 KB) - added by markjaquith 11 years ago.
24804.16.diff (8.3 KB) - added by markjaquith 11 years ago.
24804.17.diff (8.6 KB) - added by markjaquith 11 years ago.
24804.18.diff (399 bytes) - added by markjaquith 11 years ago.
24804.19.diff (1.3 KB) - added by ocean90 11 years ago.
Update help text
24804.20.diff (646 bytes) - added by nacin 11 years ago.

Download all attachments as: .zip

Change History (66)

#1 @markjaquith
11 years ago

24804.pin-to-top.diff is a first stab at pinning some of the controls to the top as you scroll.

https://dl.dropboxusercontent.com/s/u9iue0d4b69m2wb/2013-07-20%20at%2012.33%20AM.png

I decided to keep the meta in this revision, because it was simpler to just pin the entire controls box. I tried to display:none the meta, but it looks weird with that sudden blank space there. And you don't want to make the content jump at all. Could do it if we wrapped all the non-meta controls in a div to use for pinning, and then just let the meta scroll under the other controls, followed by the content. No tooltips when pinned. Dunno if they're needed. They'll be a bit of work.

#2 @markjaquith
11 years ago

24804.plus-loading-indicator-fixes.diff​ adds a fix for the issue whereby the loading indicator would not be visible if you've scrolled. Now it just pegs it right in the middle of the screen (with offsets for the menu). I like it.

@ocean90
11 years ago

#3 @ocean90
11 years ago

Looks good so far.

  • Adjusts gradients to our Coding Standards
  • Restores tickmarks in classic theme
  • Removes cruft from old UI: #diff-header (wp-admin-rtl.css), #diff-title-to strong, .comparing-two-revisions #diff-title-from (wp-admin.css)

@aaroncampbell
11 years ago

#4 @aaroncampbell
11 years ago

24804.diff incorporates .3 and also fixes an issue where the meta wasn't updating when the revisions did.

#5 @helen
11 years ago

This looks good. One more thing that I saw fly by somewhere else (maybe IRC?) - From and To are different colors in compare two mode. I don't think "To" should be blue in either mode, as it's not a link.

@nacin
11 years ago

@nacin
11 years ago

Probably missing some RTL

@markjaquith
11 years ago

@markjaquith
11 years ago

@markjaquith
11 years ago

@nacin
11 years ago

@nacin
11 years ago

#6 @markjaquith
11 years ago

In 24761:

Revisions: Pinned controls, layout tweaks, copy tweaks, misc.

  • When you scroll down the diff view, the controls will pin to the top.
  • The revisions meta view was cleaned up. Copy changes.
  • Loading indicator in the center of the screen (so it follows as you scroll).
  • Tooltips "flip" when you cross the center line, so that they don't hit the container edge and wrap for later revisions.
  • The "Restore" button's inactive state is handled on render, instead of after.
  • Make sure we always have a current revision, even if the timestamp doesn't work out on the most recent one.

See #24804. Props markjaquith, nacin, ocean90, aaroncampbell.

#7 @nacin
11 years ago

In 24763:

Fix strings in revisions UI. see #24804, [24761].

@ocean90
11 years ago

#9 @markjaquith
11 years ago

In 24769:

Revisions: RTL for [24761].

Props ocean90. See #24804.

#10 @nacin
11 years ago

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

Closing as fixed!

ocean90 said it needs an IE7 review. So re-opening #24736 for final review there.

@adamsilverstein
11 years ago

don't pin controls when scrolling in ie7

@nacin
11 years ago

#11 @nacin
11 years ago

  • Autosaves get lost visually.
  • UI is missing a cap check for "Restore".
  • Revisions metabox is sad.
  • data-restore-link="{{{ data.restoreLink }}}" is dead code.

See 24804.11.diff.

#12 @nacin
11 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

@nacin
11 years ago

#13 @nacin
11 years ago

The new revisions UI needs to work even when revisions are turned off, because it is also the autosave browser/comparer.

24804.12.diff builds in 11.diff and also:

  • Makes wp_get_post_autosave() return autosaves even when revisions are disabled.
  • Hacks in the ability to compare to the original post (parent), not just a revision, so we can compare the current post to an autosave.
  • Creates some fun logic in wp_prepare_revisions_for_js() when revisions are disabled that would be better as a wp_get_post_autosaves() function.

@markjaquith
11 years ago

More responsive slider. Less crazy with few revisions.

#14 follow-ups: @nacin
11 years ago

In 24790:

Revisions changes.

  • Eliminates the bloated Revisions meta box in favor of 'Revisions: #' in the publish box.
  • Adds ability to compare autosave to current post, when revisions are disabled.
  • Makes autosaves stand out visually, including "Restore This Autosave".

Also:

  • Adds missing capability check for restoring a revision.
  • When no revision matches the post's current modified time, avoid marking an autosave as 'current'.
  • Fixes wp_get_post_autosave() to return an autosave even when revisions are disabled.
  • Add 'check_enabled' arg to wp_get_post_revisions(); false avoids the wp_revisions_enabled() check.
  • Adds a responsive slider that is narrower for fewer versions. props markjaquith.

see #24804.

#15 @nacin
11 years ago

Things to do still:

  • wp_get_post_autosaves()
  • Proper subview for thrice-duplicated author to/from/tooltip UI

And that's it, as far as I can tell.

#16 in reply to: ↑ 14 ; follow-up: @batmoo
11 years ago

Replying to nacin:

  • Eliminates the bloated Revisions meta box in favor of 'Revisions: #' in the publish box.

I like that we're killing the metabox, but this seems like a really big change. Are we introducing some sort of way to transition users familiar with the Revisions metabox to look at the Publish one instead?

#17 @batmoo
11 years ago

  • Cc batmoo@… added

#18 in reply to: ↑ 16 @markjaquith
11 years ago

Replying to batmoo:

I like that we're killing the metabox, but this seems like a really big change. Are we introducing some sort of way to transition users familiar with the Revisions metabox to look at the Publish one instead?

My plan was to provide a new feature pointer for this.

#19 @DrewAPicture
11 years ago

  • Cc xoodrew@… added

I really like moving revisions to the Publish box in this way, it's a nice consolidation.

Unfortunately, it means I have to be that guy who says we'll need a (new) shorter no-js string, and a mention in the 'Publish Settings' help tab.

For the no-js, maybe '(Requires JavaScript)', or 'Enable JavaScript to browse'.

Yes, I'm asking for new/amended strings in RC2. And yes, it's because we just changed the UI.

@DrewAPicture
11 years ago

(Browsing requires JavaScript) for no-js

#20 @DrewAPicture
11 years ago

24804.14.diff adds a no-js label of '(Browsing requires JavaScript)'. If we wanted to go more general and reusable, we could do just '(Requires JavaScript)' or something. I'm not sure that really conveys what requires JavaScript in that context though.

I'm also fine with not touching the help tabs because we never really mentioned revisions there in the first place (there's no Publish Settings tab for CPTs) and revisions has its own help tabs anyway.

#21 follow-up: @nacin
11 years ago

I think just hiding "Browse" is sufficient for no-JS. We hide quite a few features without any comment or not. The messages are really just used when we can't really hide the feature in a pleasant way.

#22 in reply to: ↑ 21 @DrewAPicture
11 years ago

Replying to nacin:

I think just hiding "Browse" is sufficient for no-JS. We hide quite a few features without any comment or not. The messages are really just used when we can't really hide the feature in a pleasant way.

Works for me.

#23 @markjaquith
11 years ago

24804.15.diff combines the from/to view into individual from/to subviews. Haven't combined tooltips in yet.

#24 @markjaquith
11 years ago

24804.16.diff gets tooltips in on the template-sharing action.

#25 @markjaquith
11 years ago

In 24814:

Revisions: Combine our tooltip and from/to templates into one generic template

We had three copies of essentially the same template. Now just one, with minor logic inside.

Also fixes a bug where tooltip.revision was being initially set to a diff instead of a revision.

For trunk. See #24804.

#26 @markjaquith
11 years ago

In 24815:

Revisions: Combine our tooltip and from/to templates into one generic template

We had three copies of essentially the same template. Now just one, with minor logic inside.

Also fixes a bug where tooltip.revision was being initially set to a diff instead of a revision.

For 3.6. See #24804.

#27 @markjaquith
11 years ago

24804.18.diff changes the compare-two toggle text to "Compare any two revisions". Siobhan pointed out that even in "single revision" mode you are technically comparing two revisions (just that the one you're comparing to is always the previous revision).

#28 @nacin
11 years ago

  • Keywords i18n-change added

#29 @nacin
11 years ago

In 24859:

Change 'Change two revisions' to 'Compare any two revisions' for clarity. props siobhan. see #24804.

#30 @nacin
11 years ago

In 24860:

Change 'Change two revisions' to 'Compare any two revisions' for clarity. props siobhan. see #24804.

Merges [24859] to the 3.6 branch.

#31 @markjaquith
11 years ago

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

Considering this finished.

#32 in reply to: ↑ 14 ; follow-up: @pmaiorana
11 years ago

Replying to nacin:

  • Eliminates the bloated Revisions meta box in favor of 'Revisions: #' in the publish box.

Sharing some feedback from a VIP customer that I thought was apt:

"My main problem is that I can no longer see the edit trail on the same screen as I use to edit a story. So if I want to see who was involved in a post, I now have to click twice: Once to edit the post, and then again to see the revision history.

Even then, I still can’t see a list of everyone who was involved — I just have to scrub that slider left and right."

Losing the ability to see revision history at-a-glance in a multi-author setup seems like a misstep.

#33 @pmaiorana
11 years ago

  • Cc pm@… added

#34 @markjaquith
11 years ago

Paul — Yeah, we mocked up some concepts around that, but it would have been too much to get done for 3.6. Nacin is really excited about this concept: having the "compare any two revisions" mode show a list of authors in the range. We may revisit this as a 3.7/3.8 enhancement.

For WordPress.com VIP clients who need this at a glance, you could add back the old meta box code.

@ocean90
11 years ago

Update help text

#35 follow-up: @ocean90
11 years ago

Sadly we haven't updated the help text for [24860].

#36 in reply to: ↑ 32 @nacin
11 years ago

Replying to pmaiorana:

Losing the ability to see revision history at-a-glance in a multi-author setup seems like a misstep.

I think it's the right call. That said, the revisions meta box can be re-enabled via code with:

add_action( 'add_meta_boxes', 'nacin_restore_the_revisions_meta_box' );
function nacin_restore_the_revisions_meta_box( $post_type, $post ) {
    if ( wp_revisions_enabled( $post ) )
        add_meta_box('revisionsdiv', __('Revisions'), 'post_revisions_meta_box', null, 'normal' );
}

I could go for re-enabling the meta box for 3.6, given that it is hidden by default anyway. Can remove the bloat in 3.7 after revisions UI gets even better. (We'd keep Revisions: # and the "Browse" link.) Just a matter of uncommenting a function call.

@nacin
11 years ago

#37 in reply to: ↑ 35 @nacin
11 years ago

Replying to ocean90:

Sadly we haven't updated the help text for [24860].

I will ask the translators.

#38 @nacin
11 years ago

In 24921:

Update help text for [24860]. props ocean90. see #24804. for 3.6.

#39 @nacin
11 years ago

Translators asked, got a quick response from a few of the more active ones: http://make.wordpress.org/polyglots/2013/07/31/should-we-change-one-more-string-in-3-6/. Hence [24921].

#40 @nacin
11 years ago

In 24922:

Update help text for [24860]. props ocean90. see #24804. for trunk.

#41 @markjaquith
11 years ago

I'm fine with reënabling the Revisions meta box, given that it is hidden by default.

#42 @nacin
11 years ago

In 24955:

Restore the revisions meta box for 3.6. Hidden by default as before. Can be useful for a few lingering use cases. see #24804.

#43 @nacin
11 years ago

In 24956:

Restore the revisions meta box for 3.6. Hidden by default as before. Can be useful for a few lingering use cases. see #24804.

Merges [24955] to the 3.6 branch.

Note: See TracTickets for help on using tickets.