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.
Problem/Motivation
We should be able to place menu blocks from the menu page. This is a half thought-out idea. It is barely a proof-of-concept. More explanation and code to come later.
Proposed resolution
The proposed patch in #11
Remaining tasks
Reroll the patch and test.
User interface changes
The menu page would be changed.
Comment | File | Size | Author |
---|---|---|---|
#24 | allow_menu_blocks_to_be-2083871-24.patch | 160.96 KB | Nitesh Sethia |
#11 | menu-2083871-11.patch | 12.81 KB | tim.plunkett |
#8 | Screen Shot 2013-09-10 at 2.59.31 PM.png | 29.14 KB | tim.plunkett |
#8 | Screen Shot 2013-09-10 at 2.59.43 PM.png | 28.92 KB | tim.plunkett |
#5 | menu-2083871-5.patch | 3.3 KB | tim.plunkett |
Comments
Comment #1
tim.plunkettNo point in testing yet.
Comment #2
tim.plunkett#2083877: Add a theme selector to the place block form
Comment #3
tim.plunkettBlocked on #2084197: Uninstalling menu module removes all custom menus
Comment #4
tim.plunkettOkay, here's an action link. It triggers the modal added in #2083877: Add a theme selector to the place block form and then keeps you on the menu page.
Attached a patch with just this issue, and one combined.
Comment #5
tim.plunkettOkay, both blockers went in!
Comment #6
xjmNice! Patch code looks fine, but maybe the test should also confirm the block is actually placed in the expected region?
I tested manually and it seems to work as intended. Couple comments on the UX:
admin/structure/menu
? I vaguely remember discussing this but not sure with who or what the decision might have been.admin/structure/menu/manage/admin
needs hook_help(). That could give us a chance to explain about placing the block.That doesn't give me any clue as to what I just did. It would be MUCH better to say something like "Block placed in Bartik Highlighted region" or something, possibly followed by
<a href="admin/structure/block/whatever">Administer blocks for Bartik</a>.
Comment #7
tim.plunkettWe added explicit tests for that in #2083877: Add a theme selector to the place block form, which is the modal we're just repurposing.
1) Not that I know of. I don't really touch overlay...
2) I made this decision myself, because having the dropbutton operation trigger a modal seemed very odd to me
3) I have no idea if these support weights, but yes.
4) "+ Add link" is the menu_link add form. So, no.
5) Um, no idea how to add descriptions to local actions. Probably should be hook_help()
6) Would be cool!
7) Yep, sounds good.
8) That's exacerbated by this workflow, yes. Could be done separately, or here, doesn't matter to me.
Leaving at NR for usability input on 6.2, local action vs dropbutton
Comment #8
tim.plunkettOkay, this addresses 6.3, 6.7, and 6.8
Comment #9
Bojhan CreditAttribution: Bojhan commentedI will review this later today. Sorry, for the backlog on this tag.
Comment #10
xjmCool, the specific status message helps a lot.
@Bojhan, helping polish this help text would help for sure. :)
Comment #11
tim.plunketts/drupalPost/drupalPostForm
Comment #12
tim.plunkettUnassigning to help keep track of what is blocked on me.
Comment #13
jibranIssue link is updated to https://drupal.org/node/2076283
Comment #14
Bojhan CreditAttribution: Bojhan commentedSo I am looking at it, but can I have a little bit more background - on the why?
Comment #15
jessebeach CreditAttribution: jessebeach commentedKevin would like to create soft-associations between placeable things (menus, aggregation entities, views) and layout. The idea is, AFAIU, if you create something, like a menu, you probably want to use it as well. So we provide a short-circuit path to put that new thing into active use.
Comment #16
Bojhan CreditAttribution: Bojhan commentedThis is a bit of a short review, I will do a longer one tomorrow - sadly I am running out of time today. I am surprised at how ugly our menu page edit screen has gotten with the merge of edit and list of links. So I am all for this concept, we have actually advocated for this several times in the past as usability testing showed this was needed. Its all about connecting the dots.
Onto the actual UI:
The text could indeed use some work, looking at it now - I wouldn't even bother with it. We don't need text, all its doing now is telling you to go to Blocks administration, thats not needed anymore - and we don't really need to explain the UI. If we need to explain the UI, we have a #Fail UI. In general I feel like this just needs a bit more design, probably not what you wanted to hear - but it will save us a lot of time in the long run when we transfer this to other concepts and user test this.
Comment #17
Bojhan CreditAttribution: Bojhan commentedActually can we not just have a theme + region selector? Or is that not possible because of all the instance stuff?
Comment #18
tim.plunkettWe cannot allow blocks to be placed without presenting the whole form, since we don't know what required fields might be needed by the block plugin, or a hook_form_alter
Comment #19
yoroy CreditAttribution: yoroy commentedI wonder if it's useful to keep referring to a menu as a block. I.e. the action says 'Place block'. Would it be more clearer if this read 'Place menu'? Same with the messages: I'm working on a menu here, but it's being referred to as a block.
Agreed with Bojhan that the page help text can be removed alltogether.
Re. 17 & 18: when do those potentially required fields get checked? Maybe it could be a 2-step process where we do have only a theme + region selection, on submit it either is ok or we ask for more configuration as per any required field that needs it.
Where do I make suggestions for the 'place block' form itself? It's a bit messy :)
Comment #20
klonos@yoroy, regarding the rename to 'Place menu': great idea, here's an issue (credits to you) for it so that it doesn't get lost: #2100643: Replace the 'Place block' page title with either 'Place menu' or 'Place menu block'.
Comment #21
klonosComment #22
jhedstromComment #23
areke CreditAttribution: areke commentedComment #24
Nitesh Sethia CreditAttribution: Nitesh Sethia commentedRerolled the patch.
Comment #26
jhedstromThis seems like a feature request at this point in time.
Comment #27
Bojhan CreditAttribution: Bojhan as a volunteer commentedI think my review still applies.