Navigation block with active class on ul

Created on 3 August 2021, almost 3 years ago
Updated 29 May 2024, 29 days ago

Drupal Version: 9.2

Domain module version: 1.0.0-beta6

Expected Behavior

An active class on link of current domain in domain_nav_block block with the Unordered list of links (ul) option

Actual Behavior

No class

Steps to reproduce

* Place DomainNavBlock in your page
* Configure it with "Unordered list of links" (ul) option as "Link theme"
* visit homepage

✨ Feature request
Status

Needs work

Version

2.0

Component

Code

Created by

πŸ‡«πŸ‡·France SylvainM

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡§πŸ‡ͺBelgium tim-diels Belgium πŸ‡§πŸ‡ͺ

    This should be targeted towards the 2.x branch.

  • Merge request !75Add code from MR towards 1.x β†’ (Open) created by tim-diels
  • Status changed to RTBC about 2 months ago
  • πŸ‡§πŸ‡ͺBelgium tim-diels Belgium πŸ‡§πŸ‡ͺ

    I've tested the code and it works as expected, so for me it is RTBC. I created a new MR against 2.x with same code and tested it.

  • Pipeline finished with Success
    about 2 months ago
    Total: 221s
    #163659
  • Status changed to Needs work about 1 month ago
  • πŸ‡©πŸ‡ͺGermany diqidoq Berlin | Hamburg | New York | London | Paris

    Please do not set RTBC by own patches without review of others.

    Again: Thanks for the report and the work in here. Very much appreciated. But some nitpics:

    -          $items[] = ['#markup' => Link::fromTextAndUrl($label, $url)->toString()];
    +          if ($domain->isActive()) {
    +            $url->mergeOptions(['attributes' => ['class' => 'active']]);
    +          }
    +          $items[] = [
    +            '#type' => 'link',
    +            '#url' => $url,
    +            '#title' => $label,
    +          ];
    

    These changes do not only add active class in case but also change/replace the ['#markup' => Link::fromTextAndUrl($label, $url)->toString()]; without documentation in the issue comments. This should be documented and explained even if obvious for the patch provider, so that follow up issues can better be tracked down and find its root causes.

  • πŸ‡§πŸ‡ͺBelgium tim-diels Belgium πŸ‡§πŸ‡ͺ

    I only added the code that was already provided. but fair enough I follow that I can not set own patches/MR as RTBC.

    What do you need from us to continue in this issue? Some sort of CR so everybody knows what happend and that can be linked in this issue and in the release notes? Or were you thinking about something else?

Production build 0.69.0 2024