Account created on 25 November 2007, about 17 years ago
#

Merge Requests

More

Recent comments

🇩🇪Germany spuky

as described in the ticket:

When building the breadcrumb on pages one level below the home page, the module checks to see if the segment should be rendered as a link. In doing so, it checks whether the current $links array is empty and the option to show the current page as a link is selected. This means that on the first level of sub-pages the home page link will have the URL even though it is not the current page.

This described the behaviour before 2.0.7 which was fixed by the code of this issue...

🇩🇪Germany spuky

you could do the check for first loop element before to make twig look a little cleaner...

{% if breadcrumb %}
  <nav class="breadcrumb" role="navigation" aria-labelledby="system-breadcrumb">
    <h2 id="system-breadcrumb" class="visually-hidden">{{ 'Breadcrumb'|t }}</h2>
    <ol>
    {% for item in breadcrumb %}
      <li>
      {% if loop.first %}
           <a href="{{ custom_url }}">{{ item.text }}</a>
      {% else %}
        {% if item.url %}
            <a href="{{ item.url }}">{{ item.text }}</a>
        {% else %}
             {{ item.text }}
        {% endif %}
      {% endif %}
      </li>
    {% endfor %}
    </ol>
  </nav>
{% endif %}
🇩🇪Germany spuky

The normal use case for having the Breadcrumb also on the homepage...
is in my opinion to have it there for consistency off your layout... A link to the page you are already visiting does not really make sense...
(a usecase that is already not the default)

you can have links to pages you are visiting... by using "Make the current page title segment a link"
then also the Frontpage will follow that behavior... Since when you are on the Frontpage the Frontpage is the current page...
(also a use case I would only understand in special circumstances..)

Now you are already overriding the Front page url by twig with a custom_url for the first element so this is already a workaround...
since you want a different first element in the breadcrumb.
Then why should it be more of a workaround to do that for all cases it can match ? It's just your first workaround done correctly..

I don't know enough about you usecase to have a diffrent URL than the Frontpage url... there to give advice.

🇩🇪Germany spuky

Changes are in...

and I still don't get your Problem... If you are On the Homepage.. and there is a breadcrumb.. what is the point of putting out a link to the page I am on... since you got a custom Url... that you want to link to...

your twig would take into account the no link case for the first element..

{% if breadcrumb %}
  <nav class="breadcrumb" role="navigation" aria-labelledby="system-breadcrumb">
    <h2 id="system-breadcrumb" class="visually-hidden">{{ 'Breadcrumb'|t }}</h2>
    <ol>
    {% for item in breadcrumb %}
      <li>
        {% if item.url %}
          {# Check the First Breadcrumb and custom #}
          {% if loop.first %}
            <a href="{{ custom_url }}">{{ item.text }}</a>
          {% else %}
            <a href="{{ item.url }}">{{ item.text }}</a>
          {% endif %}
        {% else %}
          {# Check the First Breadcrumb and custom #}
          {% if loop.first %}
            <a href="{{ custom_url }}">{{ item.text }}</a>
          {% else %}
             {{ item.text }}
          {% endif %}
        {% endif %}
      </li>
    {% endfor %}
    </ol>
  </nav>
{% endif %}
🇩🇪Germany spuky

you should get those changes.. in the Dev version there was just nor release since they where mergeded

🇩🇪Germany spuky

looks fine on a first look you might try....

{{ custom_url|raw }}

drupal filters might clean your url from the output..

🇩🇪Germany spuky

So just to check if I understood yo correctly...

You want an Option to have a Home segment.. But it should not be Linked ?

But you are describing links not showing ?

Can you share the twig code you are using....

🇩🇪Germany spuky

Thanks for your contribution code looks plausible on a first look will takt a closer look and try when I get arraound to it.

🇩🇪Germany spuky

MR created new code style warning has nothing to do with the PR and should be fixed in an own issue

🇩🇪Germany spuky

spuky made their first commit to this issue’s fork.

🇩🇪Germany spuky

Putting back to needs review... since not applying to 2.0.6 when we are on 2.0.8 is not something that needs work ;-)

🇩🇪Germany spuky

+1 for RTBC since @vladimiraus is a mainer now I am looking forward to a release (patches work.. )

🇩🇪Germany spuky

Drupal 9 has been deprecated for since 1. November 2023 can you check if it is the case in 10.3 or 11 ?

🇩🇪Germany spuky

thanks that helps will provide them as time permits...

🇩🇪Germany spuky

I don't know if and what tests the maintainers would like... so setting to needs review.

🇩🇪Germany spuky

any body in need of that see convert to utf8 Active

on tamper

🇩🇪Germany spuky

spuky changed the visibility of the branch 3307979-feeds-tamper-plugin to hidden.

🇩🇪Germany spuky

spuky created an issue.

🇩🇪Germany spuky

spuky made their first commit to this issue’s fork.

🇩🇪Germany spuky

Updated the MR to a minimal Readme... pointing the way..

🇩🇪Germany spuky

/ start irony
Sorry for not seeking for

help on the wiki page

that is linked nowhere... but on the gitlab page...
where you first have to Seek out the wiki under Plan... then get an 404 because there is no route page ...
but discover the 3 pages on the righthand Side

i guess that is fine and everybody will find his way...
/end irony

Can we do better ?

🇩🇪Germany spuky

understand that.. got here by:

issues on my project filed by another Maintainer trying out starshot

found:
https://www.drupal.org/project/starshot

which invites to tryout the Proto type

which says it is now here...

Was a little lost on how to get the repo up and running.

So maybe a readme...

WARNING:
THIS REPO IS for DEV DO NOT BUILD PRODUCTION SITES BASED ON IT

and striped down Install instructions would help

🇩🇪Germany spuky

This issue helped me get up an running with starshot

Did some changes that i Had to figure out... while folowing the installation instructions..

+1 for rtbc

🇩🇪Germany spuky

spuky made their first commit to this issue’s fork.

🇩🇪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

🇩🇪Germany spuky

@atteshow.. could you share more about your config.. any exclusions set ?

🇩🇪Germany spuky

spuky changed the visibility of the branch 3088784-can-use-token to active.

🇩🇪Germany spuky

spuky changed the visibility of the branch 3088784-can-use-token to active.

🇩🇪Germany spuky

+ 1 for removing... MR added...

🇩🇪Germany spuky

spuky made their first commit to this issue’s fork.

🇩🇪Germany spuky

I guess this is an Olivero issue...

They seem to make Breadcrubs scrollable on mobil .breadcrumb__content has: -webkit-overflow-scrolling: touch;

Which they have not mitigated for long breadcrumbs on Desktop and the overflow-x: auto; width: max-content; on .breadcrumb__list leads to the Scrollable box for the breadcrumb...

My 2 cents would be to leave the the crumbs in their long form in the default..... (since we then don't omit any Information... )
and leave it up to the site builder to find their solution...

🇩🇪Germany spuky

On mobil but my guess would be.

First segment for form/nerd-quiz

Second segment for form/nerd-quiz/confirmation

Which both resolve to the same title

🇩🇪Germany spuky

@bbu23 thanks for bringing up your MR once Merge Dynamic breadcrumb Active is ready we will have a dependency on Token... and can work on making tokens work in more places...

🇩🇪Germany spuky

could you share a little more about what breadcrumbs are not there with which config... ?

🇩🇪Germany spuky

learned from #3467236: Path processed Urls have no corresponding route

that this is working (the code change in the MR) made it in somehow...

Tests will be done in a Testing initiatve... once a few more bugs are resolved..

So I am closing this and the MR

🇩🇪Germany spuky

setting back to active will look into this when time permits...

🇩🇪Germany spuky

Ok figured it out you must set both:

Paths to be excluded while generating segments:
\/node\/

Paths to replace with custom breadcrumbs:
regex!/node/([0-9]*/)::

🇩🇪Germany spuky
Paths to replace with custom breadcrumbs

regex!/node/([0-9]*)::

No that does not help... since it is matching on both url alias and internal path... looked good on first try... but...when checking another node with an alias it also matched..

🇩🇪Germany spuky

I would guess you where counting on a bug with the field

now the behavior is..
below an entered path there are no more segments generated (including the page title)

kat1
--kat1-1
----kat1-1-1
------kat1-1-1-1
--kat1-2
----kat-1-2-1

having that

i can use kat1\/kat1-1\/kat1-1-1

to hide everything below the path from the breadcrumb.

I was not involved in the bugfix for the feature... ( Just happend to roll the release where the change was included)
but from reading the description of excluding paths.. i would have guessed that what it does...excluding paths not segments..

so putting /node in there hides elements for everything below /node wich describes your behavior...

I see your use case... though

And there is a solution with using:

Paths to replace with custom breadcrumbs

regex!/node/([0-9]*)::

this is not easy to discover unless you know your way around that feature but should give you a first working solution

so my Suggestion would be:

We close this issue as Works as designed...
and open a Feature Request to make an easy Option to Skip Segments

And yes you are right we are missing lots of test cases (feel free to provide MRs with additional tests )

🇩🇪Germany spuky

putting back to 2.x and updating title and description to avoid future confusion

🇩🇪Germany spuky

I guess your bug is other Peoples feature...

and the change was in [3196198]

🇩🇪Germany spuky

Does need a test to not reindroduce the bug..

🇩🇪Germany spuky

after playing around with options a bit... there are config settings where: "Display the front page segment on the front page" is working...

so setting this to task... because it requires a deeper look into. possible combinations.

🇩🇪Germany spuky

just to understand that issue are you using https://www.drupal.org/project/field_tokens or is it possible to reproduce that just with using core... and EB ?

🇩🇪Germany spuky

@ W01F

could you report back if the issue still exists... so I don't have to spend time on reproducing it if it might be already fixed ?

🇩🇪Germany spuky

Hi Tobias,

I am maintaining EB since about 3 month...

before trying to reproduce your issue by myself I'd like to ask you if you know if the issue still exists. (since it is 4 years old) and it maybe has been fixed during this time...

🇩🇪Germany spuky

For everybody still using a patch workflow here is the MR as a patch so we get feedback on whats actually to be merged...

🇩🇪Germany spuky

@jonasane...

What is wrong with the MR that you rerroll 93 ? Why not download the plain diff of the MR use it as local patch... and give feedback on that...

🇩🇪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...

🇩🇪Germany spuky

is this still an issue since preferred menu option is there just asking if thats an issue for users ?

🇩🇪Germany spuky

Closing that as outdated if you are still struggeling feel free to reopen or post a new issue.

🇩🇪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...

🇩🇪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..

🇩🇪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

after running into translated list fields today I found my own MR from a year ago... setting to need review...

🇩🇪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.

🇩🇪Germany spuky

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

🇩🇪Germany spuky

spuky made their first commit to this issue’s fork.

🇩🇪Germany spuky

As I said with the EB implementation of the json+ld output everthing is working as expected for me...

just on Link in the breadcrub on the page...
json+ld with link to the current page...

but as i Said I use Metatags for other tags... but for the breadcrumb I use the Easy breadcrub option...

If you get two elements in the breadcrumb your theme or another module might also be adding the current page.

try switching to another theme like Olivero and see if the issue persists..

🇩🇪Germany spuky

What happens where ?

EB implementation of json+ld schema markup will produce expected results..

turning both options on will generate a last element with a item containing the url ...
turning only the first option on will generate a last element without the item ( google knows the URL it is on anyway)

turning both off will have the parent as last element which is also valid since google knows the URL and the Title of the page it is on anyways...

So please define the Problem if there is one.

Are you using the EB option to add Json ld (which does not need metatags module) or are you trying to add the schema markup via Metatags module ?

🇩🇪Germany spuky

it kind of depends in the original description he was talking about using the

Schema.org WebPage Submodule of Schema.org Metatag module at least i conclude that since "add to schema metatags" is not an option of EB but then It would be Schema.org WebPage that is not allowing the elements..

I guess we are still in the process of Figuring out the Problem if there is one ;-)

🇩🇪Germany spuky

@leymax #3458010 was about only allowing the last Item having no link since it is Possible with other Features like custom Breadcrumbs to introduce Elements that have no link to be added to the chain...

And since I am dog fooding dev on one of my customer sites am I got the waring about invalid json+ld markup pretty quickly...

Google Search Console was never complaining about an missing URL in the last Element of the Breadcrumb path...

it was complaining before i used the patch from #3340203 (which landed in 2.07)
and when using using Dev or a Patched version including (#3446802) before #3458010

I am planing on doing a release this week. Since DEV is working pretty stable since 2 Weeks on 3 customer sites.

🇩🇪Germany spuky

also I tried to understand your MR isn't that changing nothing but returning earlier ?

if the exeption is hit I assume $entity is still NULL so we return parent::getTitle($request, $route);

same as your MR

and in your described case the title seems still to be empty after returning

parent::getTitle($request, $route);

so the fallback of using the Path as a source for the breadcrumb Items is used...

🇩🇪Germany spuky

just to get a little more insight ist this old issue related or the same as your issue ?

https://www.drupal.org/project/easy_breadcrumb/issues/3207853 🐛 Routes with a parameter converter but no entity do not get their title inserted in the breadcrumb Needs work

🇩🇪Germany spuky

Unless you have a very special usecase (For most people it is making Google Search console happy) you could just use the:

Add current breadcrumb as structured data.

of easy breadcrumb which adds schema.org json+ld markup to the header of the page and only add the rest of the schema.org tags via the Metatag module.

🇩🇪Germany spuky
This issue can be closed now.

why should the issue be closed... it was not about the 2 Stars...
but about "HTML5 Validation Prevents Submission in Tabs" the thing to do would be to update the mr to match patch 148 and set the issue back to needs review or RTBC

🇩🇪Germany spuky

I'd like to understand this a little more..

Google allows a last Item without a link and the current page must not be in the navigation path. So its maybe an issue in the Schema.org WebPage Submodule of Schema.org Metatag module.

The json+ld Option of Easy Breadcrumb generates the Schema markup so is there a reason to use.. Metatags module instead of Easy Breadcrumbs...

🇩🇪Germany spuky

@berramou you might want to open an issue in the EB issue queue. And attach your MR there so it gets more eyeballs and also it might be easier for folks to try it out. By just adding the diff as a patch to a composer.json. It is still on my todo list... to check it out closer... but i did not get around doing it.. noticed it has an merge conflict with the latest dev version...

🇩🇪Germany spuky

i also see the double stars that Andreic is seeing after applying mr57 against 3.6 so setting to needs work..

🇩🇪Germany spuky

same here... getting an 500 Error on /quickedit/attachments?_wrapper_format=drupal_ajax

ArgumentCountError: Too few arguments to function Drupal\embed\EmbedType\EmbedTypeBase::__construct(), 3 passed in /var/www/html/web/modules/composer/entity_embed/src/Plugin/EmbedType/Entity.php on line 56 and exactly 4 expected in Drupal\embed\EmbedType\EmbedTypeBase->__construct() (Zeile 29 in /var/www/html/web/modules/composer/embed/src/EmbedType/EmbedTypeBase.php).

it results in an "Oops, something went wrong. Check your browser's developer console for more details." on every page as long as you have quickedit permissions..

since Ck editor is in the call stack ... i guess it is the same route cause...

reverted back to 1.7 for the time being..

🇩🇪Germany spuky

can not reproduce with latest dev Version... feel free to reopen if it still is an issue

🇩🇪Germany spuky

Should be fixed in the latest dev version if not feel free to reopen

🇩🇪Germany spuky

spuky changed the visibility of the branch 2990464-breadcrumb-markup-should to hidden.

🇩🇪Germany spuky

Did move the patch +93 into an new mr

And adapted the Test "testGetTitleString" to assert for Drupal\Core\Render\Markup instead of String.

Would be nice to get that in after this long time... Would love to have at least a few RTBCs before merging.

Production build 0.71.5 2024