Problem/Motivation

<nolink> menu items that don't have a URL are rendered as <a href="">link title</a>

Steps to reproduce

Add a link to the main menu, or any menu that uses the navbar nav pattern, with a URL of <nolink>

Proposed resolution

Update the navbar_nav pattern to have an if statement on {{ item.url }}

Remaining tasks

  • [x] Fix it

User interface changes

N/A

API changes

N/A

Data model changes

N/A

CommentFileSizeAuthor
#10 Capture d’écran du 2024-02-19 15-49-45.png7.71 KBgrimreaper

Issue fork ui_suite_bootstrap-3422210

Command icon 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:

About issue forks

Comments

thejimbirch created an issue. See original summary.

thejimbirch opened merge request !166

thejimbirch’s picture

thejimbirch commented
Issue summary: View changes
Status: Active » Needs review

Merge request created that updates the navbar_nav pattern.

To validate:

  • Apply the change.
  • Create a main menu link that the URL is <nolink>
  • Save
  • View the menu source and verify the menu item is wrapped in a <span> and no longer an <a>
thejimbirch’s picture

thejimbirch commented
Issue summary: View changes
grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Assigned: Unassigned » grimreaper
grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Assigned: grimreaper » pdureau
Status: Needs review » Needs work

@thejimbirch, thanks for the bug report and the MR.

@pdureau, I have assigned it to you to get your feedback after my code review. Then I will implement the solution.

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Assigned: pdureau » grimreaper

Discussed with @pdureau.

No button case to handle. Printing link_attributes. I will test for navbar-text automatic or not. In favor of not placed automatically if no visual difference.

Grimreaper credited pdureau.

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Assigned: grimreaper » Unassigned
Status: Needs work » Needs review
StatusFileSize
new Capture d’écran du 2024-02-19 15-49-45.png7.71 KB

link_attributes rendered.

No need to add the navbar-text class automatically as it only change the text color. But as you can on the screenshot, it is not aligned with normal links.

So I would say if someone wants to have a nolink or a button, he/she is able to add additional classes using menu_link_attributes or something else.

@thejimbirch, I let you test the MR before merging. I will merge on thursday, in 2 days if no feedback is provided.

thejimbirch’s picture

thejimbirch commented
Status: Needs review » Reviewed & tested by the community

Code looks good. Nice catch with the attributes.

Marking RTBC

Thanks!

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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