"active" location style CSS doesn't work (totally)

[sbardian]

"active" location style CSS doesn't work (totally)

[davidkartuzinski]

Yes totally okay to close issue! Consider it done.

[sbardian]

@davidkartuzinski so in version 9.0.1 useClassNames is gone. Since I'm not doing any inline css anymore there was no longer a need for it. It just removed inline css and let you style via the class names. Let me look into what you are experiencing with the active link.

[sbardian]

@davidkartuzinski as far as I can tell, if you take the trailing / off the url it matches and applies the breadcrumb__link__active class. gatsby-plugin-breadcrumb uses Gatsby's (Reach Router's) <Link/> component. The to prop has to match the url to be considered "active". I think there are levels of acceptable matching we could play with, if we would like it to match less restrictively, but I think that is why the trailing / in the url wasn't giving you the active class on the breadcrumb. . .

let me know if you find the same behavior.

[davidkartuzinski]

@sbardian

Yes yes, that is exactly the issue. the "/" is not always at the end of the URL which is why I am seeing different behaviour. Clicking around my site, some of my URLs have the trailing slash and some don't (ie. Tags and Categories) and when the trailing slash is not there, the active Class works great.

I can see how to have all my URLs have the trailing slash. I don't know how to easily remove the trailing slash which is generated automatically by the onCreateNode function (I suppose createFilePath is more accurate). From the gatsby-node.js.

I think personally I would like to standardise this through my site by having trailing slashes every time. But I think the average programmer isn't going to notice, read the docs (if we added it) and just end up blaming the plugin. Which is bad.

So I conclude that allowing for either scenario is probably a good idea for DX.

EDIT: If you wanted to be the ultimate "Rock Star" you could provide an option for either scenario or both. And I assume careful programmers will pay attention and settle on one scenario.

-DK

[sbardian]

@davidkartuzinski let me look into what I might be able to do on the plugin end. If you are in a hurry you might be able to use gatsby-plugin-remove-trailing-slashes(https://www.gatsbyjs.org/packages/gatsby-plugin-remove-trailing-slashes/). This would remove all the trailing slashes of your paths so your urls match up. This used to be a requirement of the plugin, maybe I could mention this in the docs if people have trailing slashes in their app? I will also see if I can add a prop for Reach Routers getProps option.

[sbardian]

@davidkartuzinski I think we are already good. Try adding this function to the code where you are using the <Breadcrumb> component.

  const isPartiallyActive = ({ isPartiallyCurrent, isCurrent }) => {
    return isPartiallyCurrent && isCurrent
      ? { className: "breadcrumb__link breadcrumb__link__active" }
      : {}
  }

Then pass the following prop to the <Breadcrumb> component:
getProps={isPartiallyActive}

<Breadcrumb
   crumbs={crumbs}
   crumbLabel={crumbLabel}
   getProps={isPartiallyActive}
/>

Or just use the gatsby-plugin-remove-trailing-slashes in your case. I'm going to work on updating the README in my plugin to mention these options.

[sbardian]

@davidkartuzinski see the updated README Gotchas. Let me know what you think.

[davidkartuzinski]

@sbardian - Thank you for your attention to this. I definitely think the const isPartiallyActive to be a bit "hacky" and would rather not.

In fact the actual problem is this piece of code:

exports.onCreateNode = ({ node, getNode, actions }) => {
  const { createNodeField } = actions
  if (node.internal.type === `Mdx`) {
    const slug = createFilePath({ node, getNode, basePath: `pages` })
    createNodeField({
      node,
      name: `slug`,
      value: slug,
    })
  }
}

This generates a slug with a trailing slash and is use in two major places. The blog roll and tags/categories. The other links like menu or prev/next, etc are being generated from the frontmatter and can or cannot have the trailing slash.

Therefore, you made me think and I came up with this const slug = createFilePath({ node, getNode, basePath: pages }).replace(/\/$/,"")

This removes the trailing slash from the slug and the full function now looks like this:

exports.onCreateNode = ({ node, getNode, actions }) => {
  const { createNodeField } = actions
  if (node.internal.type === `Mdx`) {
    const slug = createFilePath({ node, getNode, basePath: `pages` }).replace(
      /\/$/,
      ""
    )
    createNodeField({
      node,
      name: `slug`,
      value: slug,
    })
  }
}

I then removed all trailing slashes I had put in earlier haha, and now the activeClass works perfectly everywhere! https://gatsby-theme-naked.netlify.com/

I think most people are using the original function as it's in the docs and most of the themes I looked it and maybe this could also be an option or perhaps THE option if people are running into this situation.

This is the least amount of code.

Anyway, I don't have the overview you have, and maybe I am the only one with this issue!? In either case, thank you. And I have already started on the post for the community from you.

-DK

[davidkartuzinski]

I forgot to mention, I think your Readme is super clear. And I wouldn't change a word. (Except to add the above if you think it's valuable)? @sbardian

[sbardian]

@davidkartuzinski yeah the getProps prop is part of the <Link> API from Reach Router so not much we can do about how it works (other than creating better functions for the logic it performs).

I think with all the different ways to create pages/and their URLs in gatsby, everyone could end up with one result or another. So adding the gotcha and the info there should help people decide on a path to take, just as you have. Update the URLs on pages they are creating themselves, use one of the other options on things they don't create/have control over/use gatsby-plugin-remove-trailing-slashes/etc..

Thanks for working on the blog post! Can't wait to see it.

Are we ok to close this issue out?