Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

docs(router): NavLink wording improvements #10380

Merged
merged 6 commits into from
Apr 2, 2024
Merged

docs(router): NavLink wording improvements #10380

Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
32413b9
docs(router): NavLink wording improvements
Tobbe Mar 31, 2024
706c349
Merge branch 'main' into tobbe-nav-link-wording
Tobbe Apr 1, 2024
cd3f3bf
Merge branch 'main' into tobbe-nav-link-wording
Tobbe Apr 2, 2024
0f4fc36
Apply suggestions to add colons from code review
Tobbe Apr 2, 2024
2cbf271
Expand on doc comments
Tobbe Apr 2, 2024
84d8f74
Merge branch 'main' into tobbe-nav-link-wording
Tobbe Apr 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions docs/docs/router.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ Named route functions simply return a string, so you can still pass in hardcoded

## Active links

`NavLink` is a special version of `Link` that will add an `activeClassName` to the rendered element when it matches **exactly** the current URL.
`NavLink` is a special version of `Link` that will add an `activeClassName` to the rendered element when it matches the current URL.

```jsx title="MainMenu.jsx"
import { NavLink, routes } from '@redwoodjs/router'
Expand All @@ -218,7 +218,7 @@ import { NavLink, routes } from '@redwoodjs/router'
const MainMenu = () =>
<ul>
<li>
<!-- When match "/" -->
<!-- Will match "/" -->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original wasn't clear, but I think the spirit of it is lost if we replace it with just "will". I think "when" was supposed to be short for something like adds "activeLink" when the URL matches "/". Maybe we could do will add "activeLink" when the URL matches "/"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to keep as close as possible to the existing docs, just make them easier to read and understand.

With that in mind I like your first suggestion best. It's probably more inline with what the original author wanted to express. So I'll go with that for this PR.

That said, it's actually wrong now. But I'll get this merged, and then follow up with a different PR that explains what I mean.

Copy link
Member Author

@Tobbe Tobbe Apr 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow up here: #10401

<NavLink
className="link"
activeClassName="activeLink"
Expand All @@ -227,7 +227,7 @@ const MainMenu = () =>
</NavLink>
</li>
<li>
<!-- When match "/?tab=tutorial" (params order insensitive) -->
<!-- Will match "/?tab=tutorial" (params order insensitive) -->
<NavLink
className="link"
activeClassName="activeLink"
Expand All @@ -238,12 +238,12 @@ const MainMenu = () =>
</ul>
```

Alternatively, you can add the `activeMatchParams` prop to your `NavLink` to match the current URL **partially**
The `activeMatchParams` prop can be used to control how query params are matched
Tobbe marked this conversation as resolved.
Show resolved Hide resolved

```jsx
import { NavLink, routes } from '@redwoodjs/router'

// Will render <a href="/?tab=tutorial&page=2" className="link activeLink"> when on any of Home tutorial pages
// Will render <a href="/?tab=tutorial&page=2" className="link activeLink"> when on any Home tutorial page
const MainMenu = () => (
<li>
<NavLink
Expand All @@ -260,7 +260,7 @@ const MainMenu = () => (

> Note `activeMatchParams` is an array of `string` _(key only)_ or `Record<string, any>` _(key and value)_

More granular match, `page` key only and `tab=tutorial`
More granular match; needs to be on the tutorial tab (`tab=tutorial`) and have the `page` key specified
Tobbe marked this conversation as resolved.
Show resolved Hide resolved

```jsx
// Match /?tab=tutorial&page=*
Expand Down
Loading