Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.On some pages (for example, "do you really want to do XYZ" confirm forms), the "add to shortcuts" link looks confusing and out of place. See the attached screenshot (inspired by #609122: shortcut_set_save() no longer deletes existing links when a new set of links is passed in), which shows a particularly confusing example...
It would be nice if we had some good way to hide those on a page by page basis.
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | shortcut-hide.patch | 1.81 KB | casey |
| #8 | shortcut-hide.patch | 1.85 KB | casey |
| #6 | shortcut-hide.patch | 1.75 KB | casey |
| add-shortcut-confirm-form.png | 28.89 KB | David_Rothstein | |
Comments
Comment #1
jim0203 CreditAttribution: jim0203 commentedThere doesn't seem to me to be anything intrinsic to particular pages to say whether they should have an "add to shortcuts" link - so it is overly simplistic to add a line near the top of shortcut_page_build() like:
Where shortcut_check_link() is a new function which returns TRUE if the link should be displayed and FALSE otherwise? Then all the different conditions could be bundled into that function.
Comment #2
webchickThis is still an issue. I think the wrapper function makes sense, I'm just not sure what would go inside it. Confirmation forms, for sure. What else?
Comment #4
Bojhan CreditAttribution: Bojhan commentedConfirmation forms
Admin/config/system/shortcut/
All top level sections
That's all I could think of, the last is somewhat questionable but apart from Dashboard and Content I see no reason why one would duplicate these links.
Comment #5
bleen CreditAttribution: bleen commentedsubscribe
Comment #6
casey CreditAttribution: casey commentedComment #8
casey CreditAttribution: casey commentedComment #9
bleen CreditAttribution: bleen commentedtagging ... I'll volunteer to write the tests once we have a working patch
Comment #10
David_Rothstein CreditAttribution: David_Rothstein commentedLooks like a good start! Although since showing the button is the most common case, I think the logic should be reversed - i.e., I would call the function shortcut_hide_add_remove_button().
And for this:
I realize this is hard to detect, but I don't think we want to assume a particular location in the $page array, or base it off the theme function - both of those can easily be different.
To solve this, can we put this in an #after_build function attached to all forms, perhaps? (The overlay module does this, for example.) Then, to identify the confirm form, maybe something as simple as modifying the confirm_form() function to set
$form['#confirm_form'] = TRUE, which gives us a clean thing to check for. I think there are other modules besides this one that might want to react to a confirm form, so this would be of general use.Otherwise, I think this makes sense - it is probably the best we're going to be able to do. I wonder if there are some confirm forms where it wouldn't make sense to do this, but can't think of any off the top of my head, and in the worst case, they can always find a way to add it back via the API function that the patch provides...
Comment #11
casey CreditAttribution: casey commentedI think we should only hide the shortcut button if the page main content is a confirm_form (not when there is a confirm_form somewhere else on the page).
Comment #12
David_Rothstein CreditAttribution: David_Rothstein commentedSome modules (e.g. Panels) might have a different concept of the "main page content" than Drupal core does, so I don't think we should hardcode it to an area of the page array like that.
From the user's perspective, what is a scenario where a confirm form would appear on the page but not be the main intended focus of the page? I can't think of too many situations like that ... that's why I suggested just determining it based on the presence of the form anywhere on the page. (I guess there are AJAX-y scenarios where that might happen, but if the confirm form is being added via AJAX, then it won't really affect us here anyway.)
Comment #14
Bojhan CreditAttribution: Bojhan commentedLets won't fix this, I dont think its likely users will do this purposely anyway - its only a side affect of the badly placed shortcut icon.