Problem/Motivation

Just as node/%node is a parent item for view, edit, delete so should node/%node/revisions/% be the parent for view, revert, delete.
Currently, you need to revert from the revision list page, where you only have the date to know whether it's the revision that you actually want to switch to.

Proposed resolution

Show revision actions on view revision page.

Remaining tasks

1. Get UX opinion.
2. Ifusing actions, rename issue title.

Contributor tasks needed
Task Novice task? Contributor instructions Complete?
Reroll the patch if it no longer applies. Instructions
Update the issue summary Instructions
Update the issue summary noting if allowed during the beta Instructions

User interface changes

Adds a revert, and delete button to the current revision.

API changes

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task because it's needed for usability.
Issue priority Normal.
Unfrozen changes Unfrozen because it only adds buttons to make reverting to a version more convenient.
Prioritized changes The main goal of this issue is usability.
Disruption Seems to be isolated to revisions.
CommentFileSizeAuthor
#34 1108888-34.patch616 byteslokapujya
#27 node-1108888-26.patch2.17 KBmrded
#27 interdiff-1108888-24-26.txt946 bytesmrded
#25 node-1108888-24.patch1.99 KBmrded
#25 interdiff-1108888-22-24.txt422 bytesmrded
#22 node-1108888-22.patch1.44 KBmrded
#22 interdiff-1108888-16-22.txt2.32 KBmrded
#18 drupal-node_rev_local_tasks_after_comment_16-110888.png35.08 KBrooby
#18 drupal-node_rev_local_tasks_after_comment_5-110888.png35.04 KBrooby
#18 drupal-node_rev_local_tasks_after_comment_4-110888.png35.08 KBrooby
#18 drupal-node_rev_local_tasks_before-110888.png32.87 KBrooby
#16 node_revisions-1108888-16.patch1.51 KBpetercook
#16 revsion_tabs.png12.44 KBpetercook
#5 1108888-node-revisions-rev4.patch1.52 KBbrianV
#4 1108888-node-revisions-rev3.patch1.51 KBbrianV
#3 1108888-node-revisions-rev2.patch1.44 KBbrianV
#1 1108888-node-revisions.patch1.71 KBboombatower

Comments

boombatower’s picture

boombatower commented
Assigned: Unassigned » boombatower
Status: Active » Needs review
StatusFileSize
new 1108888-node-revisions.patch1.71 KB
boombatower’s picture

boombatower commented

Not sure if delete should be included since it isn't for node/%node/delete.

brianV’s picture

brianV commented
StatusFileSize
new 1108888-node-revisions-rev2.patch1.44 KB

Re-rolled against current head. I left delete included, although I agree that it's removal should be considered for consistency.

brianV’s picture

brianV commented
StatusFileSize
new 1108888-node-revisions-rev3.patch1.51 KB

re-rolled with bugfix.

brianV’s picture

brianV commented
StatusFileSize
new 1108888-node-revisions-rev4.patch1.52 KB

Here's another option for consideration - rather than making the revisions actions local tasks, I put them as local actions.

Status: Needs review » Needs work

The last submitted patch, 1108888-node-revisions-rev4.patch, failed testing.

brianV’s picture

brianV commented
Status: Needs work » Needs review

#4: 1108888-node-revisions-rev3.patch queued for re-testing.

brianV’s picture

brianV commented

#5: 1108888-node-revisions-rev4.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 1108888-node-revisions-rev4.patch, failed testing.

brianV’s picture

brianV commented
Status: Needs work » Needs review

#4: 1108888-node-revisions-rev3.patch queued for re-testing.

brianV’s picture

brianV commented

#5: 1108888-node-revisions-rev4.patch queued for re-testing.

yesct’s picture

yesct commented

#5: 1108888-node-revisions-rev4.patch queued for re-testing.

welly’s picture

welly commented
Status: Needs review » Reviewed & tested by the community

Patch applies correctly.

welly’s picture

welly commented

#5: 1108888-node-revisions-rev4.patch queued for re-testing.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Needs review
Issue tags: +Needs screenshots

Hm, can someone take a screenshot of what this does? I can't quite parse it from the description, nor from reading the code.

petercook’s picture

petercook commented
StatusFileSize
new revsion_tabs.png12.44 KB
new node_revisions-1108888-16.patch1.51 KB

Changed two cases of MENU_LOCAL_ACTION in patch to MENU_LOCAL_TASK
Shorter tab labels would be good with hover over for extra description.

Status: Needs review » Needs work

The last submitted patch, node_revisions-1108888-16.patch, failed testing.

rooby’s picture

rooby commented
Status: Needs work » Needs review
Issue tags: -Needs screenshots
StatusFileSize
new drupal-node_rev_local_tasks_before-110888.png32.87 KB
new drupal-node_rev_local_tasks_after_comment_4-110888.png35.08 KB
new drupal-node_rev_local_tasks_after_comment_5-110888.png35.04 KB
new drupal-node_rev_local_tasks_after_comment_16-110888.png35.08 KB

Here are before and after shots:

Before:
drupal-node_rev_local_tasks_before-110888.png

After the patch in #4:
drupal-node_rev_local_tasks_after_comment_4-110888.png

After the patch in #5:
drupal-node_rev_local_tasks_after_comment_5-110888.png

After the patch in #16:
drupal-node_rev_local_tasks_after_comment_16-110888.png

After seeing these images, a quick comparison of the patches in #4 & #16 indicates that #16 is a re-roll of #4.

mradcliffe’s picture

mradcliffe
he/him
Primary language English
commented
Status: Needs review » Needs work

Reverting to needs work as the patch fails tests, and it may need a re-roll.

Thank you for the screenshots, rooby.

rooby’s picture

rooby commented

So what is the desired appraoch, actions (#5) or tabs (#16)?

Seems to me that actions makes more sense, because the labels describe actions.

mrded’s picture

mrded commented
Assigned: boombatower » mrded
mrded’s picture

mrded commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-1108888-16-22.txt2.32 KB
new node-1108888-22.patch1.44 KB

Take a look new patch, it should works fine.

rbayliss’s picture

rbayliss commented

The patch applies now, but I'm getting the following Exception after creating a node:

Symfony\Component\Routing\Exception\ResourceNotFoundException: The route for '/node/1/revisions//revert' could not be found in Drupal\Core\Routing\RouteProvider->getRouteCollectionForRequest()

Commenting out the revert and delete hook_menu entries makes this go away, so it is breaking on building the local tasks. I'll leave at needs review for the moment we can get the testbot error log.

Status: Needs review » Needs work

The last submitted patch, node-1108888-22.patch, failed testing.

mrded’s picture

mrded commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-1108888-22-24.txt422 bytes
new node-1108888-24.patch1.99 KB

This version should be better.

rbayliss’s picture

rbayliss commented

Patch applies, and the tabs look good. One problem is that when you go to node/{NID}/view or node/{NID}/revisions/{VID}/view, you get a page not found. I think there will need to be a way to deal with menu default local tasks in routes, but that problem is not introduced by this patch.

mrded’s picture

mrded commented
StatusFileSize
new interdiff-1108888-24-26.txt946 bytes
new node-1108888-26.patch2.17 KB

I've added description and made title more obvious.

Status: Needs review » Needs work

The last submitted patch, node-1108888-26.patch, failed testing.

rooby’s picture

rooby commented

We still need to agree on the outcome of #20.

I feel actions are more logical.

mrded’s picture

mrded commented

I don't see a reason to show 'MENU_LOCAL_ACTION' without any 'MENU_LOCAL_TASK', and it looks strange.
What's why, I vote for 'MENU_LOCAL_TASK'.

rocketeerbkw’s picture

rocketeerbkw commented
Issue summary: View changes
Related issues: +#2270717: There is no way to get back from viewing an old node revision

It was mentioned in #2270717: There is no way to get back from viewing an old node revision that tasks and actions could be used in these ways:

View current | Revisions
<< Previous | Revision 1196 | Revert to this | Delete this | Next >>

View current | View draft | Edit draft | Revisions
<< Prev | Revision 1196 | Revert to this | Delete this | Next >>

yesct’s picture

yesct commented
Assigned: mrded » Unassigned
Issue summary: View changes
Issue tags: +Needs issue summary update, +Needs reroll

It's been a long time. unassigning so it is clear anyone can work on this.

it's a normal task, we should see if it can be done during the beta. adding the instructions for beta evaluation to the summary.

rooby’s picture

rooby commented

Again, we still need a consensus on #20

My vote is for actions instead of tabs because deleting and reverting are actions.

lokapujya’s picture

lokapujya
Primary language English
Location Boston
commented
Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs issue summary update, -Needs reroll
StatusFileSize
new 1108888-34.patch616 bytes

Add the actions. I think you can get back to the current revision by clicking the title.

lokapujya’s picture

lokapujya
Primary language English
Location Boston
commented
Issue summary: View changes
lokapujya’s picture

lokapujya
Primary language English
Location Boston
commented
Issue tags: +Needs usability review, +Usability
lokapujya’s picture

lokapujya
Primary language English
Location Boston
commented
Title: Make node/%node/revisions/%/* local tasks » Add actions and/or tabs to a node's revision view
johnpitcairn’s picture

johnpitcairn commented

Getting back to the current revision by clicking the title may work, but that is likely to be rather non-obvious to the average user, isn't it? I'd advocate an obvious link for that...

Bojhan’s picture

Bojhan commented
Status: Needs review » Closed (won't fix)
Issue tags: -Needs usability review

Nope, this violates our guidelines http://drupal.org/ui-standards

rooby’s picture

rooby commented

@Bojhan:

Any chance of getting some more specifics on the parts of the guideline that this doesn't align with?

Do you mean that neither option (tabs or actions) are valid within the guideline?

Since the way this currently works is (IMO) not very user friendly I have to wonder a little why we have UI standards that don't allow making something more user friendly.