[gatsby-theme-notes] breadcrumb home icon is linking to “/“ instead of basePath.

[jakewies]

Description

The root breadcrumb, specified in the gatsby-theme-notes options section as homeText, is always linking to “/“, even when a user passes a custom basePath

Steps to reproduce

1. Bootstrap a quick example with gatsby-theme-notes:

gatsby new my-themed-notes https://github.com/gatsbyjs/gatsby-starter-notes-theme

cd my-themed-notes

2. In gatsby-config.js, pass a custom basePath value to the options property:

// gatsby-config.js

module.exports = {
  plugins: [
    {
      resolve: `gatsby-theme-notes`,
      options: {
        basePath: "/notes",
      },
    },
  ]
  // …
}

3. Run yarn develop

4. Navigate to localhost:8000/notes/example-dir.

Expected result

When clicking the root breadcrumb icon, ~, I should be re-directed back to the notes basePath, i.e. localhost:8000/notes.

Actual result

I am re-directed to /, i.e. localhost:8000.

Environment

❯ gatsby info --clipboard

  System:
    OS: macOS 10.14.5
    CPU: (8) x64 Intel(R) Core(TM) i7-4770HQ CPU @ 2.20GHz
    Shell: 5.3 - /bin/zsh
  Binaries:
    Node: 11.14.0 - /usr/local/bin/node
    Yarn: 1.15.2 - /usr/local/bin/yarn
    npm: 6.9.2 - ~/.npm-packages/bin/npm
  Languages:
    Python: 2.7.16 - /usr/local/bin/python
  Browsers:
    Chrome: 77.0.3833.0
    Firefox: 65.0.1
    Safari: 12.1.1
  npmPackages:
    gatsby: ^2.13.6 => 2.13.6
    gatsby-theme-notes: ^1.0.0 => 1.0.0
  npmGlobalPackages:
    gatsby-cli: 2.7.3

I’ve tracked the issue to the BreadcrumbHome component. It is hard-coding / as the value of the Link component’s to prop. A simple fix would be to pass this component the basePath as a prop.

Again, this may be the intended behavior, but in my brain it seems that the root breadcrumb should always go back to the root of the notes section. I’d be happy to submit a PR if this should be the case. 😄

[lannonbr]

@gatsbyjs/themes-core, what does everyone think about this? I am kinda leaning to have the default be mapped to the root of the notes instead of the root of the site always. And if someone wants to make a link in the notes index to go up to the homepage of their site if the two are not the same, it could be easily shadowed to do such.

It's the thing that I think in cases like this, navigation for the notes theme should be encapsulated and not exit the theme's context by default

[johno]

Agreed, this should definitely link to the notes theme basePath. 👍

[jakewies]

@lannonbr my thoughts exactly. By linking to the root of notes, instead of the root of the site, gatsby-theme-notes is truly encapsulated/abstracted. Embracing the intended nature of themes 😄

[lannonbr]

Alright. So I would say then feel free to push a PR up for this @jakewies. In the BreadcrumbHome component, you should be able to use the use-options.js hook to get the basePath and then plop it in to the the link.

[jakewies]

Awesome! I'll get on it 👍

I have been struggling to test changes made in the gatsby/themes directory while working on another PR for an issue I found in gatsby-theme-notes.

I brought this up to @johno in that issue. It would be super helpful to get some guidance on this, as the attempts I've made to test changes in that directory haven't worked.

You can check out more info on the issue here, but the gist of it is that running a theme-starter in gatsby/themes, and then making changes to a theme that the starter depends on does not update the starter in the browser. It looks like the starter is using the node_modules version of the theme instead of the theme in the yarn workspace.

[gatsbot]

Hiya!

This issue has gone quiet. Spooky quiet. 👻

We get a lot of issues, so we currently close issues after 30 days of inactivity. It’s been at least 20 days since the last update here.

If we missed this issue or if you want to keep it open, please reply here. You can also add the label "not stale" to keep this issue open!

As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contributefor more information about opening PRs, triaging issues, and contributing!

Thanks for being a part of the Gatsby community! 💪💜

[jakewies]

@johno I'll make a PR for this now that I have the ability to see changes to themes in development.

[jakewies]

I just pushed a commit in #16564 to fix this issue, and it works as intended. But something else came to mind.

If I, a user of gatsby-theme-notes, pass a custom basePath:

// gatsby-config.js

module.exports = {
  plugins: [
    {
      resolve: `gatsby-theme-notes`,
      options: {
        basePath: `/notes`,
      },
    },
  ],
}

Then the constructed breadcrumb will look like this:

~ / notes / example-dir

The problem with this is that now you have a home breadcrumb ~, which links to /notes, as well as a notes breadcrumb, which links to /notes. The intended behavior might be better off as:

~ / example-dir

Where only 1 breadcrumb represents the base path, the home breadcrumb ~. I hope this makes sense.

What do you all think?

EDIT: This would probably make #16556 unnecessary?