| Comment | File | Size | Author |
|---|
Issue fork diff-2954352
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
| Comment | File | Size | Author |
|---|
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
oknateWhen you're viewing a diff, the tabs to return to viewing the node or editing the node disappear.
This patch adds tabs on the route diff.revisions_diff.
Comment #3
oknateComment #4
akalam commentedThe patch #2 worked for me on Drupal 9.4.5, seems like a simpler solution compared to #3169729: No local tasks on compare revisions page
Comment #5
sokru commentedAdded also tests to make sure the local tabs are visible.
Comment #6
programmerdiego commentedPatch #5 worked for my Drupal 9.5.11 site.
I added the following to the composer.json file
"patches": {
"drupal/diff": {
Displays local tasks on revisions sub-tab: "https://www.drupal.org/files/issues/2023-10-05/2954352-diff-show-local-t..."
}
}
Comment #7
tanmayk+1 for RTBC
Comment #11
acbramley commentedGreat and simple improvement. Thanks!
Comment #12
acbramley commentedUnfortunately this has caused issues on Node revision pages when there is more than one secondary tab (which is what is required to actually render the link)
Comment #15
sokru commented@acbramley could you provide a bit more information about the issue you have faced? I'm not being able to reproduce it on route
diff.revisions_diffeg./node/1/revisions, when selecting also "Show primary tabs" on "Secondary tabs" blocks.Comment #16
acbramley commented@sokru you need to have at least 1 other secondary tab to trigger it.
Comment #17
liam morlandThis patch is a port of the patch in #3169729: No local tasks on compare revisions page. It should avoid the problem mentioned in #12.
Comment #18
acbramley commentedPlease use a merge request, patch files are not accepted on this project anymore.
Comment #20
liam morlandMerge request with patch #17.
There are also other changes to get phpstan to pass again.
Comment #21
adamzimmermann commentedI'm using
2.0.0-beta3and the changes in MR 64 work perfectly for me and solved a client request. Thank you!So RTBC from that perspective, but I see that this MR targets the 8.x branch, so I'll refrain from updating the issue Status for now.
Comment #22
liam morlandRebased on 2.x.
Comment #23
sokru commentedThis needs tests. I'd recommend adding tests from https://www.drupal.org/files/issues/2023-10-05/2954352-diff-show-local-t...
and separate test for issue reported on #12.
Comment #24
adamzimmermann commentedI have a commit ready to add the tests, but I don't seem to have access to the MR.
I'm not sure we need a test for the issue reported in #12 since the new approach doesn't have that issue, and from my experience of trying to do it the wrong way too (before I found this issue), the fact that we can load the node edit page earlier in the test proves there isn't an issue.
EDIT: Not sure why I didn't see the get push access button before, but I found it and was able to push my commit.
Comment #25
adamzimmermann commentedComment #26
sokru commentedThis still needs work for issue mentioned on #12 (and #16). So it needs steps to reproduce, fix for that and tests to make sure it won't get regression.
Comment #27
liam morlandThe merge request avoids the issue in #12 and it has a test.
Comment #31
acbramley commentedComment #32
acbramley commentedI've tested the new approach and while it does fix the issue in #12 I'm not sure the performance overhead of this is something I'd be keen to commit to.
I debugged through and that alter hook is run on every single page load of a revision comparison page. None of the static caching for the tasks or route information works since we're not on the canonical route which means it's doing a lot of work to get those tasks, we also don't have test coverage for the bug in #12.
To reproduce it, you just need another secondary task under Revisions, e.g we have this so the secondary tabs render when on a Revisions route.
Core has a similar pattern with Content moderation to render an Overview secondary tab on admin/content so the Moderated content tab shows.