- Merge request !11Issue #2834253: Missing column headings in Revisions list → (Merged) created by Liam Morland
- First commit to issue fork.
- 🇳🇿New Zealand ericgsmith
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.
- 🇨🇦Canada Liam Morland Ontario, CA 🇨🇦
Patch containing the current state of the merge request.
- 🇨🇦Canada Liam Morland Ontario, CA 🇨🇦
Sorry, ignore the patch in #22. This is the correct patch containing the current state of the merge request.
- 🇨🇦Canada Liam Morland Ontario, CA 🇨🇦
@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 ✨ Add "compare revisions" button on top for long form. Needs review .
- Status changed to Needs work
about 1 year ago 12:28am 20 May 2024 - Status changed to Needs review
about 1 year ago 3:10pm 21 May 2024 - Status changed to RTBC
about 1 year ago 7:18pm 21 May 2024 - Status changed to Needs work
about 1 year ago 10:48pm 21 May 2024 - 🇦🇺Australia acbramley
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.
- 🇳🇿New Zealand ericgsmith
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.
- 🇦🇺Australia acbramley
@ericgsmith source and target definitely make more sense to me. I'd be happy with that.
- Status changed to Needs review
about 1 year ago 1:11pm 22 May 2024 - 🇨🇦Canada Liam Morland Ontario, CA 🇨🇦
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.
- Status changed to Needs work
about 1 year ago 3:56am 31 May 2024 - Status changed to Needs review
about 1 year ago 2:16pm 31 May 2024 - 🇨🇦Canada Liam Morland Ontario, CA 🇨🇦
Rebased. All comment threads resolved.
- 🇦🇺Australia acbramley
Sorry should've mentioned this needs to go into 2.x.
- Status changed to Needs work
6 months ago 2:25am 17 December 2024 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- 🇨🇦Canada Liam Morland Ontario, CA 🇨🇦
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. - 🇮🇳India sandip
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.
- 🇮🇳India sandip
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.
- Status changed to RTBC
10 days ago 2:44am 29 May 2025 -
acbramley →
committed 95c18f23 on 2.x authored by
liam morland →
Issue #2834253 by liam morland, sandip, acbramley, bkosborne, ginovski,...
-
acbramley →
committed 95c18f23 on 2.x authored by
liam morland →
- 🇨🇦Canada Liam Morland Ontario, CA 🇨🇦
Would you cherry-pick this onto 8.x-1.x? It applies cleanly.
- 🇦🇺Australia acbramley
@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.
- 🇨🇦Canada Liam Morland Ontario, CA 🇨🇦
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.
- 🇨🇦Canada Liam Morland Ontario, CA 🇨🇦
Patch ported to 8.x-1.x for Composer patching.
-
acbramley →
committed 66362c9c on 8.x-1.x authored by
liam morland →
Issue #2834253 by liam morland, sandip, acbramley, bkosborne, ginovski,...
-
acbramley →
committed 66362c9c on 8.x-1.x authored by
liam morland →