Problem/Motivation

The Add new @node_type or Log in to... local action on forum.index (/forum) and forum.page (/forum/TID) are not wrapped in a <ul> markup.

Forum listing page

Forum thread

Proposed resolution

Update the ForumController::buildActionLinks to wrap local links in a <ul>.
This solution is the same workaround used in class BlockLibraryController::buildLocalActions.

Remaining tasks

  1. Write a patch
  2. Review

User interface changes

Local actions link on forum are correctly wrapped in a ul markup.

API changes

None

CommentFileSizeAuthor
#26 HTML-AddForumTopicBtn.png133.79 KBpaulocs
#24 interdiff_17-24.txt917 bytesraman.b
#24 2828680-24.patch1.85 KBraman.b
#18 Screenshot-from-2020-09-15-12-30-05.png82.71 KBamietpatial
#17 after-patch-forum.png222.97 KBbandanasharma
#17 forum_missing_ul_local_action-2828680-17.patch1.68 KBbandanasharma
#16 Screen Shot 2020-07-22 at 6.06.48 PM.png333.13 KBtanubansal
#16 Screen Shot 2020-07-22 at 6.06.12 PM.png338.24 KBtanubansal
#12 forum_missing_ul_local_action-2828680-12.patch1.69 KBdrclaw
#9 forum-thread.jpg87.23 KBdamontgomery
#9 forum-page.jpg93.09 KBdamontgomery
#2 forum_missing_ul_local_action-2828680-1.patch1.68 KBmogtofu33
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mogtofu33 created an issue. See original summary.

mogtofu33’s picture

mogtofu33 CreditAttribution: mogtofu33 as a volunteer and at Catalyst IT commented
FileSize
forum_missing_ul_local_action-2828680-1.patch1.68 KB
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented

Is there a #theme menu_local_actions we could put on the container?

Chi’s picture

Chi CreditAttribution: Chi commented
Related issues: +#1853072: Remove forum_menu_local_tasks_alter() hack and instead add links in ForumController::build

I just filed a similar issue which however is not related to this one because forum action links come directly from controller. Back in Drupal 7 the links were generated in hook_menu_local_tasks_alter(), which still exists in Drupal 8.
#2829997: Local action links should be inside UL element

I would move the links back to actions area but leave the "disallowed" message in place since it is not an action and not even a link.

$links['disallowed'] = [
  '#markup' => $this->t('You are not allowed to post new content in the forum.'),
];

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ commented
Status: Needs review » Needs work

#3: There is no theme for local actions that I could find.

The patch no longer applies.
We should get screenshots for this error as well.

damontgomery’s picture

damontgomery CreditAttribution: damontgomery commented
Issue summary: View changes
FileSize
forum-page.jpg93.09 KB
forum-thread.jpg87.23 KB
damontgomery’s picture

damontgomery CreditAttribution: damontgomery commented

I updated the issue summary with screenshots using 8.5. I didn't need to create any content. There is no UL, but the functionality seems ok.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

drclaw’s picture

drclaw CreditAttribution: drclaw at Fuse Interactive commented
Version: 8.6.x-dev » 8.8.x-dev
Status: Needs work » Needs review
FileSize
forum_missing_ul_local_action-2828680-12.patch1.69 KB

Here's a patch re-roll against 8.8.x

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

enzipher’s picture

enzipher CreditAttribution: enzipher commented

Patch in #12 works fine for me. Thanks!

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

tanubansal’s picture

tanubansal CreditAttribution: tanubansal at Salsa Digital commented
FileSize
Screen Shot 2020-07-22 at 6.06.12 PM.png338.24 KB
Screen Shot 2020-07-22 at 6.06.48 PM.png333.13 KB

ul wrapper is missing on forum local at drupal 9.1. Is there any patch for 9.1?

bandanasharma’s picture

bandanasharma CreditAttribution: bandanasharma as a volunteer and at gai Technologies Pvt Ltd for gai Technologies Pvt Ltd commented
FileSize
forum_missing_ul_local_action-2828680-17.patch1.68 KB
after-patch-forum.png222.97 KB

Re-roll the patch for d9 and also added after patch image.

amietpatial’s picture

amietpatial CreditAttribution: amietpatial as a volunteer and at gai Technologies Pvt Ltd for gai Technologies Pvt Ltd commented
Status: Needs review » Reviewed & tested by the community
FileSize
Screenshot-from-2020-09-15-12-30-05.png82.71 KB

#17 works fine on drupal 9.1, after and before screenshot attached.

tanubansal’s picture

tanubansal CreditAttribution: tanubansal at Salsa Digital commented

Tested the same on 9.1, working fine
this can be moved to RTBC

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented 
+++ b/core/modules/forum/src/Controller/ForumController.php
@@ -323,15 +322,24 @@ protected function buildActionLinks($vid, TermInterface $forum_term = NULL) {
           ],
+          // Without this workaround, the action links will be rendered as <li>
+          // with no wrapping <ul> element.
+          '#prefix' => '<ul class="action-links">',
+          '#suffix' => '</ul>',
         ];
       }

I think we should open a follow-up to see if we can find a better approach for this problem.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented
Issue tags: +Needs followup

+1 on #20.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Needs work

Let's do that before this actually goes in.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

raman.b’s picture

raman.b CreditAttribution: raman.b at OpenSense Labs commented
Status: Needs work » Needs review
Issue tags: -Needs followup
FileSize
2828680-24.patch1.85 KB
interdiff_17-24.txt917 bytes

Created #3181052: Find a better approach to wrap local action links in a <ul>

paulocs’s picture

paulocs
Primary language Portuguese, Brazil
Location Belo Horizonte
CreditAttribution: paulocs at CI&T commented
Assigned: Unassigned » paulocs
paulocs’s picture

paulocs
Primary language Portuguese, Brazil
Location Belo Horizonte
CreditAttribution: paulocs at CI&T commented
Assigned: paulocs » Unassigned
Status: Needs review » Reviewed & tested by the community
FileSize
HTML-AddForumTopicBtn.png133.79 KB

Patch #24 looks good.
The patch has a documentation that mentions the follow-up that was pointed on comment #20.

Moving to RTBC.

  • catch committed 290d404 on 9.2.x 
    Issue #2828680 by raman.b, bandanasharma, mogtofu33, drclaw,...

  • catch committed ec02947 on 9.1.x 
    Issue #2828680 by raman.b, bandanasharma, mogtofu33, drclaw,...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Version: 9.2.x-dev » 9.1.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 9.2.x and cherry-picked to 9.1.x, thanks!

I'll unpostpone the follow-up.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.