Make WordPress Core

Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#35029 closed defect (bug) (fixed)

Remove title attributes: the revisions limit in the Publish box

Reported by: afercia's profile afercia Owned by: afercia's profile afercia
Milestone: 4.5 Priority: normal
Severity: normal Version: 4.4
Component: Revisions Keywords: has-screenshots, has-patch, commit, title-attribute
Focuses: ui, accessibility Cc:

Description

See related #24766 and all the following tickets about title attributes.

When a revisions limit is set using WP_POST_REVISIONS and a post has reached the revisions limit, a title attribute is used in the Publish box to convey this info. Notice this title attribute is used only when the limit is reached.

https://cldup.com/CLosTY_Cx8.png

If this is considered a relevant information to convey, then maybe there's something lacking in the interface :) I'd recommend to find a simple way to display it as plain text in the box. If not relevant, should be simply removed.

Personally, I'd lean toward the first option. Needs some UI feedback.

Attachments (5)

35029.patch (1.7 KB) - added by afercia 8 years ago.
35029.2.patch (2.0 KB) - added by afercia 8 years ago.
35029.3.patch (1.8 KB) - added by afercia 8 years ago.
35029.4.patch (1.8 KB) - added by afercia 8 years ago.
35029.5.patch (1.5 KB) - added by afercia 8 years ago.

Download all attachments as: .zip

Change History (43)

#1 @afercia
8 years ago

  • Owner set to afercia
  • Status changed from new to assigned

@afercia
8 years ago

#2 @afercia
8 years ago

  • Keywords has-patch has-screenshots added

First pass, would need the help of some native English speakers for a better wording. I'd just suggest to keep the sentence as short as possible, see the second example on the right.

https://cldup.com/H_-0lxEr1g.png

#3 @afercia
8 years ago

Also, not sure if the "+" is really necessary, I'd lean toward removing it and further simplify the code.

This ticket was mentioned in Slack in #design by afercia. View the logs.


8 years ago

#5 follow-up: @joedolson
8 years ago

What about this for the text:

Revisions (3): Browse
Your site is limited to 3 revisions

That communicates the concept better to me.

#6 @karmatosed
8 years ago

I like the right screenshot better, I'm keen on the shorter sentence. Maybe "Limit for revisions is set to 3" or "Revisions have a limit of 3"?

I'm not keen on the 3+. Either we show the true value or we just don't show a figure. We're saying underneath the limit anyway.

Last edited 8 years ago by karmatosed (previous) (diff)

#7 in reply to: ↑ 5 @melchoyce
8 years ago

Replying to joedolson:

What about this for the text:

Revisions (3): Browse
Your site is limited to 3 revisions

That communicates the concept better to me.

This is my favorite proposal so far.

Style-wise, I think the italic, slightly lighter text works.

#8 @afercia
8 years ago

I agree the shorter the better.
The "Browse" link text would need some context and maybe we could use the current screen-reader-text and simplify the markup:

[link]Browse revisions (3)[/link]

As per the limit info, I'd lean for "Limit for revisions is set to 3" because it says it's something related to configuration. "Your site is limited to 3 revisions" doesn't tell me "limited by who? by what? and most of all: why?" :) It's probably easier to translate and keep it short in other languages too, e.g.:

"Il limite per le revisioni è impostato a 3"

Either we show the true value or we just don't show a figure

Yep, the "+" is a bit confusing, I'd propose to remove it.

@afercia
8 years ago

#9 @afercia
8 years ago

Second pass. Simplified markup, lots of red in the patch. :) Screenshot:

https://cldup.com/9dJe9pNjhJ.png

#10 follow-up: @SergeyBiryukov
8 years ago

In the latest screenshot, the revisions line seems visually inconsistent with the other labels and links in the box.

I'd prefer to keep the current markup, as in https://cldup.com/H_-0lxEr1g.png, for consistency.

@afercia
8 years ago

#11 in reply to: ↑ 10 ; follow-up: @afercia
8 years ago

Replying to SergeyBiryukov:

I'd prefer to keep the current markup

Sure. Should the limit information be always displayed even when the limit is not reached? I'd say yes.

Refreshed patch:

  • removes the "+"
  • uses "Limit for revisions is set to 3"
  • always show the limit info

Screenshot:

https://cldup.com/HtzZEj32K4.png

#12 in reply to: ↑ 11 ; follow-up: @SergeyBiryukov
8 years ago

Replying to afercia:

Should the limit information be always displayed even when the limit is not reached? I'd say yes.

I agree.

I kind of like the current string, 'Your site is configured to keep only the last %s revisions', because it makes it clear that it's an intentional override and not the default value, which might be easier in terms of support.

But that's just me :) 35029.3.patch looks good otherwise.

#13 in reply to: ↑ 12 @afercia
8 years ago

Replying to SergeyBiryukov:

Replying to afercia:
I kind of like the current string, 'Your site is configured to keep only the last %s revisions', because it makes it clear that it's an intentional override and not the default value

Totally agree. We should strive to craft a sentence with the same information but... shorter! :) Ideally should fit in just one line. Thinking also to languages that typically have longer translated strings, e.g. German. cc @ocean90

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


8 years ago

#15 @ocean90
8 years ago

I'd agree with @SergeyBiryukov, maybe "Configured limit for revisions is set to 3.". But actually it's not really a limit, we only keep the last X revisions.

#16 @afercia
8 years ago

@ocean90 how long would that string be in German?

#17 @afercia
8 years ago

Need to be a shorter string... was thinking something like:

Configured to keep 3 revisions

(and it's obvious they're the last ones). In my language (and other ones) that would be longer but probably would fit in just one line:

Configurate per conservare 3 revisioni

#18 follow-up: @SergeyBiryukov
8 years ago

FWIW, I don't see a problem with a longer string.

The majority of users do not have the limit and won't see the string. Those who do have the limit should see an informative notice without have to guess what it means :)

That said, how about "Your site keeps only the last 3 revisions"?

#19 in reply to: ↑ 18 @afercia
8 years ago

Replying to SergeyBiryukov:

Those who do have the limit should see an informative notice without have to guess what it means :)

Yep, you're right :) So, I'd propose to keep the original string for now. It can always be changed later if someone will be able to find a shorter and informative sentence. Will omit the final dot for consistency with other "howto" strings used in the other postboxes in this screen.

Recap of the proposed strings so far:

Your site is configured to keep only the last 3 revisions
The revisions limit is set to 3
Your site is limited to 3 revisions
Limit for revisions is set to 3
Configured limit for revisions is set to 3
Configured to keep 3 revisions
Your site keeps only the last 3 revisions

Worth noting that when there's also an autosave, the revisions number will not match the limit number because the former counts also the autosave. This is a bit confusing and should probably be addressed in a separate ticket. See screenshot below where users will see "Revisions 4" and then "keep only the last 3 revisions":

https://cldup.com/F9hLP-9YZn.png

@afercia
8 years ago

#20 @afercia
8 years ago

  • Keywords commit added

Screenshots with the refreshed patch applied:

https://cldup.com/NJiZT9hhKh.png

#21 @afercia
8 years ago

Related: #35171.

#22 @adamsilverstein
8 years ago

  • Component changed from Administration to Revisions

#23 @afercia
8 years ago

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

In 36053:

Accessibility: Remove the revisions limit title attribute from the Publish box.

The revisions limit warning is now always displayed in plain text even when the
limit is not reached yet. Removes the plus "+" from the revisions count number.

Fixes #35029.

#24 @ryan
8 years ago

Now that I see this, I want to know how to change the limit. AFAIK, revision limit provisioning is not exposed in UI. A plugin or wp-config editing is required.

#25 @joedolson
8 years ago

I have the same concern as Ryan; since this setting isn't in the default UI, I'm not sure it's helpful to describe it as a configuration - it's not configurable without special knowledge about what methods are available to do that.

#26 @afercia
8 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

@ryan @joedolson yep you're right. There are probably a few different options to consider. Off the top of my head:

  • show the message only to admins
  • show a different message to admins and not admins
  • remove the message entirely

#27 @joedolson
8 years ago

I think it's just as problematic for admins as for anybody else; that's not something that an admin can change unless they know how to manually toggle a database option or what plug-ins to install that can handle this, and a significant percentage of WP admins would probably not know that.

I think that the only valuable information is how many revisions are currently stored, and even that is of debatable value. I think more information can be made available on the revisions screen; it doesn't need to be crammed into this context.

#28 follow-up: @afercia
8 years ago

Thanks @joedolson :) So, thinking also at #35171 and the performance issue handled in #34560, I'd be inclined to completely remove the revisions count and the warning message from the Publish box and use just "Revisions: Browse".
Detailed information can be moved to the Revisions screen. Moving the revisions limit warning there wouldn't solve the lack of a UI control to change the limit though. At least, the Revisions screen offers more room to provide instructions and maybe also a link to the Codex/Handbook (https://codex.wordpress.org/Revisions#Revision_Options).

I don't have a strong opinion about exposing the revisions limit option in the UI, but if it's something worth considering it should probably go in a separate ticket. Any thoughts more than welcome.

#29 @adamsilverstein
8 years ago

"Revisions: Browse" would be fine; we still need a check to see if revisions are enabled and that there are revisions browse before offering the link.

#30 @westi
8 years ago

  • Keywords needs-patch added; has-patch commit removed

Refreshing the workflow keywords to reflect the current state of the ticket.

#31 in reply to: ↑ 28 @afercia
8 years ago

Replying to afercia:

thinking also at #35171 and the performance issue handled in #34560, I'd be inclined to completely remove the revisions count and the warning message from the Publish box and use just "Revisions: Browse".
Detailed information can be moved to the Revisions screen.

Any objection to the above? Considering also the Revisions UI will go under a redesign process :)
https://make.wordpress.org/design/2016/02/05/revisions-ui-focus-kick-off-post/

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


8 years ago

#33 @afercia
8 years ago

Discussed this during today's a11y bugscrub on Slack and considering also the Revisions UI is going under a redesign process, our proposal would be to completely remove this info from the Publish box. @ryan's point is a very good argument: this text warns users about something they can't change because there's no control exposed in the UI for that.

In order to correctly inform users about what this limit exactly is, how they can change it editing wp-config.php, etc., there's the need of a more detailed, longer, explanation, and the Publish box is not the right place for this. The Revisions screen is probably a better place, we'd recommend to take this into consideration during the redesign process.

New patch coming soon.

@afercia
8 years ago

#34 @afercia
8 years ago

  • Keywords has-patch commit added; needs-patch removed

The new patch in 35029.5.patch removes the Revisions limit warning from the Publish box.

#35 follow-up: @adamsilverstein
8 years ago

@afercia I support removing the revisions count here.

I have one question about current patch: looking at the code, it looks like we would still see a link to 'Browse Revisions' even if the site has revisions disabled and/or no revisions are available for this post. Shouldn't we be hiding that?

#36 in reply to: ↑ 35 @afercia
8 years ago

Replying to adamsilverstein:
@adamsilverstein thanks for checking, hm not sure... the whole thing is wrapped within a check

if ( ! empty( $args['args']['revisions_count'] ) ) : ?>

that should avoid to print out the link when there are no revisions, unless I'm missing something. Do you think that would be OK?

#37 @afercia
8 years ago

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

In 36612:

Accessibility: Remove the revisions limit warning from the Publish box.

After [36053] don't show a warning about something users can't change
because there's no control exposed in the UI for that. The Revisions
screen is probably a better place to show a more complete information.

Fixes #35029.

#38 @afercia
7 years ago

  • Keywords title-attribute added
Note: See TracTickets for help on using tickets.