mmistakes / minimal-mistakes

:triangular_ruler: Jekyll theme for building a personal site, blog, project documentation, or portfolio.
https://mmistakes.github.io/minimal-mistakes/
MIT License
12.47k stars 25.71k forks source link

Breadcrumbs has a wrong url at lastest position. #3097

Open FavorMylikes opened 3 years ago

FavorMylikes commented 3 years ago

Environment

Expected behavior

Steps to reproduce the behavior

  1. Open ucas.io/dates/2021/07/
  2. The href should be /dates/2021 image

Other

I have read the code below.

https://github.com/mmistakes/minimal-mistakes/blob/0491cd362bd8e742940c1f4eaae89cf9da6ef7e7/_includes/breadcrumbs.html#L17-L38

I'm new on liquid, and have not totally understand what it is doing.

But it looks like split current page url with /

When i = 2, it will generate <a> tag with href.

But I don't see where the href handler is.

iBug commented 3 years ago

Could consider it a bug. The breadcrumb was not carefully designed to handle multiple levels of hierarchy. It could only do one level (Home → Level 1 Hierarchy → Article).

ultimape commented 2 years ago

My content has many nested directories. I am also new to liquid, but managed to get this going for me.

This would be a breaking change on people's format so I'm not sure how to best implement it. And given my experience here there may be a more elegant way to code this. But you (or a maintainer) are welcome to have at it.

I rewrote that part of breadcrumbs.html page to use a different url rendering loop based on this example on stackoverflow by (@jhvanderschee)

I also set this up to always render the root breadcrumb even on the root directory / homepage. By changing

https://github.com/mmistakes/minimal-mistakes/blob/2632ff650a6efb0d856a37d675be5f1b63692181/_layouts/single.html#L11

to {% if site.breadcrumbs %}

I figure that you can always disable the breadcrumb on the root by setting the breadcrumbs: false in frontmatter instead of it being specialcase'd in the code. Not sure if that works with this bit of logic tho.

A friend suggested I could use {% capture %} to clean this up, but I'm not sure how.

This is what I changed it to to get the correct urls.

    {% comment %} thanks to https://stackoverflow.com/a/32004173/42082 for how to fix urls {% endcomment %}
    {% assign crumbs = (page.url | remove:'/index.html' | split: '/') %}
    {% assign i = 1 %} 
    <li itemprop="itemListElement" itemscope itemtype="https://schema.org/ListItem">
      <a href="{{ '/' | relative_url }}" itemprop="item"><span itemprop="name">{{
          site.data.ui-text[site.locale].breadcrumb_home_label | default: "Home" }}</span></a>
      <meta itemprop="position" content="1" />
    </li>
    {% for crumb in crumbs offset: 1 %}
      {% assign i = i | plus: 1 %}
    <span class="sep">{{ site.data.ui-text[site.locale].breadcrumb_separator | default: "/" }}</span>
    <li {% if forloop.last %}class="current"{% endif %} itemprop="itemListElement" itemscope itemtype="https://schema.org/ListItem">
      <a href="{{ '' | relative_url }}{% assign crumb_limit = forloop.index | plus: 1 %}{% for crumb in crumbs limit: crumb_limit %}{% if forloop.last %}{{ crumb }}{% else %}{{ crumb | append: '/' }}{% endif %}{% endfor %}"
        itemprop="item"><span itemprop="name">{{ crumb | replace: '-', ' ' | replace: '%20', ' '  | remove:'.html' }}</span></a>
      <meta itemprop="position" content="{{ i }}" />
    </li>

    {% endfor %}

Hope this helps!

ultimape commented 2 years ago

I noticed There is a PR in the pipeline that would make my aforementioned changes of disabling breadcrumbs on the '/' directory a reality. #3096