Merge Dynamic breadcrumb

Created on 10 August 2024, 5 months ago
Updated 13 August 2024, 5 months ago

Following of 📌 Merge into Easybreadcrumb? Needs review , i create this issue that maintainers has more visibility.
Thanks to @spuky comment https://www.drupal.org/project/dynamic_breadcrumb/issues/3421458#comment... 📌 Merge into Easybreadcrumb? Needs review

Here is the MR link https://git.drupalcode.org/project/easy_breadcrumb/-/merge_requests/99

✨ Feature request
Status

Active

Version

2.0

Component

Code

Created by

🇫🇷France berramou

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

Merge Requests

Comments & Activities

  • Issue created by @berramou
  • First commit to issue fork.
  • Merge request !131Resolve #3467372 "Merge dynamic breadcrumb" → (Open) created by spuky
  • 🇩🇪Germany spuky

    spuky → changed the visibility of the branch 2.x to hidden.

  • Status changed to Needs review 5 months ago
  • 🇩🇪Germany spuky

    @beramu I merged your feature Branch into the issue fork of this issue...

    So we have the normal workflow...
    Hope getting to to review it closer this week

    Setting to needs review for anybody that wants to get involved.

  • Status changed to Needs work 5 months ago
  • 🇩🇪Germany spuky

    Ok gave it a Testdrive in one of my dev sites..

    It did not change the output.

    but replacement code was executed...

    I guess you would have to operate on the $links array before the original

    return $breadcrumb->setLinks($links)

    since /core/lib/Drupal/Core/Breadcrumb/Breadcrumb.php line 46 has an exception "Once breadcrumb links are set, only additional breadcrumb links can be added."

    i guess it is not possible to change the breadcrumb from inside the Builder class... after calling setLinks on it...

    also spotted a Typo... $entity_bundle_from_ink should be $entity_bundle_from_link i guess.

  • 🇩🇪Germany spuky

    @bermaru Forget about:

    It did not change the output.

    but replacement code was executed...

    I guess you would have to operate on the $links array before the original

    return $breadcrumb->setLinks($links)

    since /core/lib/Drupal/Core/Breadcrumb/Breadcrumb.php line 46 has an exception "Once breadcrumb links are set, only additional breadcrumb links can be added."

    i guess it is not possible to change the breadcrumb from inside the Builder class... after calling setLinks on it...

    I forgot that we have a preprocess function running on the breadcrumb..

  • 🇫🇷France berramou

    Hello @spuky thank you for the tests and review,
    It's working in my local env, how did you test, how can i reproduce the warning ?

  • 🇩🇪Germany spuky

    i used a ddev version of one of my customer sites and added the MR as a patch to composer json...:

    "patches": {
     "drupal/easy_breadcrumb": {
                    "Test MR:131": "https://git.drupalcode.org/project/easy_breadcrumb/-/merge_requests/131.diff"
                }
    }
    

    (the plain diff link next to the MR)

    configured one node type in the dynamic breadcrumbs sektion. And used the site...

    For the Taxonomy:
    my vocabulary is website_catalog in the config the name gets stored as website-catalog so the case for the warning is Taxonomy Vocs with a space in the name (or a machine name containing an _ underscore...

  • 🇫🇷France berramou

    To test dynamic breadcrumbs,
    you need to create two content of the selected bundle i will give an example with article content

    1. Configure dynamic breadcrumbs for article for example article - [current-date:html_date]
    2. Create first article for example Article test1 with alias /article-1
    3. Create a second article for example Article test2 with alias /article-1/article-2
    4. Now if you visit the article test2 you will see that the breadcrumbs like this Home > article - 2024-08-15 > Article 2

    This is what dynamic breadcrumbs does, it changes the generated part of the configured content in breadcrumb.

  • 🇩🇪Germany spuky

    Added a suggestion that should fix The warnings on Tayonomy pages mentioned in #9 and https://git.drupalcode.org/project/easy_breadcrumb/-/merge_requests/131#...

    and maybe fix the same errors with entity that have an underscore in the machine name... so $bundle_name is only uses for classes but $bundle_key to save the config...

  • 🇫🇷France berramou

    Yes you are right we need to check if entity_types not configured no need to apply DB build.
    Thank you for the fix i accept you suggested changes.

  • 🇫🇷France berramou

    I just added the check to apply dynamic breadcrumbs only if there is an entity types configured.
    TODO:

    1. Add tests for dynamic breadcrumbs part
    2. Add AJAX to the Entity Types configuration to display the appropriate settings.
    3. Adapt dynamic breadcrumb hook_requirements set the right version instead of 2.0.6
  • 🇫🇷France berramou

    Still TODO:

    1. Add tests for dynamic breadcrumbs part
  • 🇩🇪Germany spuky

    I gave it another test drive... The Ajax makes config really better...

    I have one thing that could make it a little touch better...

    will comment on the MR

  • 🇫🇷France berramou

    It seems good idea @spuky !

Production build 0.71.5 2024