Missing column headings in Revisions list

We could add Revision 1 / Revision 2 (or simply just Revision)?

We removed it because by removing it, the column gets narrow and relationship between collection more clear.

For accessibility reasons we could semantically readd it, but i would still suppress it with CSS, so no visual difference.
That said, "Revision" or "Revision 1" and "Revision 2" are adding more confusion than removing it. You select a revision and you know that. And the button "Compare" at the end of the form is self-explaining. We could try to improve it to "Compare selection".

Full accessibility support though is a different topic, we should look at the radio button labels in context of WAI-ARIA.

I'm highly open for better proposals.

Changed to 'Compare selection'.
Noticed that in AdminFormsTest, there is a fail because of revision done in different timestamp, Edit: It was failing locally only.

Here a first reference about accessibility
http://webaim.org/techniques/forms/advanced
Sure, can go into a follow-up imho.

Added followup #2838915: Improve compare button to "Compare selection", returning this issue to active.

Even with the button changed to "Compare selected revisions", we had an editor very confused by this. They did not understand the relationship of the radio buttons to the comparison and was confused why they could not select items from the same column, and were forced to pick diagonally. It's a bit jarring if you have a long list of revisions since you don't even see the "Compare selected revisions" without scrolling down the page.

I think what should probably be done is a different approach entirely. Use a single column of checkboxes, label the column "Compare", and disable the checkboxes after two have been selected with JavaScript.

Another option is to not use the table interface for comparing at all. Instead just use two select dropdowns at the bottom of the page for the "left comparison" and "right comparison" that lists all the revisions.

Another option, what about column labels "A" and "B"? I think that helps imply the comparison purpose of the columns without taking up space. But I guess downside here is that it may only make sense for English.

This patch adds column headers A and B (not translated, since I think that may not work well for other languages?) and adds a table caption. Both of these changes improve accessibility for screen reader users and also sighted users.

I don't think that anything like A / B would improve the situation as long as there is no explanation.

The "Compare" button should really be sticky so a user doesn't have to scroll down to the end of the page...
I see that Olivero(? or at least Gin) is coming up with sticky buttons for the primary page CTA. That will help a lot.

A completely different approach with checkboxes might look visually easier. But the semantics of a checkbox don't allow us to express a "you can only choose two". If we ever rework this, then based on the best semantics and considering accessibility from the beginning. Research about other CMSes and best practices could help here.

The diff view has many other UX problems. The default selection of the revisions totally doesn't make sense in multilingual environment with draft revisions... But i don't know where to start as this feature is rarely requested by our customers recently and i think polishing will easily eat days.

This is a nice improvement. @bkosborne I might consider changing column headers A/B with "Select revision A"/"Select revision B", would be more intuitive I think. Might also consider changing the first column header from "Revision" to "Revision information".

Updated the patch to use more descriptive column headers. Also updated the table caption.

@miro_dietiker ticket #3183380 implements the Compare button at the top of the page if there are over 5 revisions. This would be nice to have included in next release along with the other accessibility patches.

This worked well for us!

Rebased the MR.

We are using this patch and editors found it an improvement of the diff screen - I note there hasn't been much discussion on this since #11 so not setting to RTBC, although I can say I have one client happy using this approach.

Edit: Ignore this patch.

Sorry, ignore the patch in #22. This is the correct patch containing the current state of the merge request.

@miro_dietiker: Perhaps the column labels should be "Compare revision A" and "Compare revision B" or "Compare from" and "Compare to".

The question of the compare button on long forms is addressed in #3183380: Add "compare revisions" button on top for long form..

PHPStan is failing.

Rebased. phpstan passes.

+1 to the latest MR.

I'm not a huge fan of these column headings, and as stated already in the comments the A/B labelling may not make sense in other languages. With that being said, I don't really have any good alternatives but don't want to commit something before we get more of a consensus.

What about using "Source" and "Target"?

I've been using Gitlabs compare revisions a lot - and I know the context and UI is different, but I feel like maybe the same label be adapted?

E.g left column = "Source", right column = "Target"

"Source" and "Target" then with something to explain it in the help text link:

"When comparing selected revisions the changes are shown as if the source revision was being updated to the target revision."

I have a few clients who often use this screen, I can ask them to review options if we can get a few other ideas in the mix.

@ericgsmith source and target definitely make more sense to me. I'd be happy with that.

Perhaps it should be "Compare" instead of "Select" to make it clear that these are used for comparisons. I have rebased and updated the labels along these lines.

For other languages, these are translatable strings, so it could be whatever is desired in that language.

See MR comments.

Rebased. All comment threads resolved.

Sorry should've mentioned this needs to go into 2.x.

Rebased

Hi @liam morland,

I have replicated the issue and applied the patch you provided, confirmed the issue is solved, however, would it be possible to display the radio buttons at the center of the column. Please see images for reference.

Thanks,
Jake

The uncentered radio buttons

Working on it.

Please review it once.

Thanks for the patch.

Those CSS selectors work to center the radio buttons, but they will change other tables too. It is probably needed to add .diff-responsive-table to the selectors.

NW for #40.

Updated css file but i did not find .diff-responsive-table class in table instead it is .diff-revisions.

Hello @sandip poddar,

Thank you for your contribution to resolving the issue. After applying your changes locally and testing the implementation, I found that the issue is not fully resolved as expected.

I have attached before and after screenshots for your reference, which highlight the areas where the issue persists or behaves unexpectedly. Based on this, I have moved the issue to the "Needs Work" state.

Please review the feedback and the screenshots to make the necessary adjustments. Let me know if you need further clarification or if you'd like to discuss any of the observations.

I am looking into it.

The issue was caused by an inline display: block; style applied to the radio button input. I have resolved this by adjusting the styling accordingly. Please review the changes and let me know if any further modifications are needed.

Hello @sandip poddar,

Thank you for your contribution to resolving the issue. After applying your changes locally and testing the implementation, I found that the issue is working as expected. So I am moving it into RTBC.

Thanks all, this is looking great

Would you cherry-pick this onto 8.x-1.x? It applies cleanly.

@liam morland I'm more inclined to mark 8.x-1.x unsupported but I want to get a stable 2.x release out first. I don't see any reason to use 8.x-1.x anymore.

It would be wonderful if this fix were cherry-picked and a final 8.x-1.9 release made with that and the commits already made. We're on 8.x-1.x until there is a full release of 2.x.

Patch ported to 8.x-1.x for Composer patching.

Done https://www.drupal.org/project/diff/releases/8.x-1.9