Skip to content

Always show Versions menu, though disabled when necessary#338

Merged
chalin merged 1 commit intomainfrom
304-versions-menu-ux-update
Jun 1, 2021
Merged

Always show Versions menu, though disabled when necessary#338
chalin merged 1 commit intomainfrom
304-versions-menu-ux-update

Conversation

@nate-double-u
Copy link
Copy Markdown
Contributor

Now we always display the Versions menu but have made it inactive on pages where it doesn't apply.

fixes #304.

Now we always display the Versions menu but have made it inactive on pages where it doesn't apply

Signed-off-by: Nate W <4453979+nate-double-u@users.noreply.github.com>
@nate-double-u nate-double-u requested a review from chalin May 28, 2021 00:31
@nate-double-u
Copy link
Copy Markdown
Contributor Author

Deploy preview: https://deploy-preview-338--etcd.netlify.app/

Copy link
Copy Markdown
Contributor

@chalin chalin left a comment

Choose a reason for hiding this comment

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

Thanks. Let's refactor this so that the dropdown html is always present, but the anchor has the added disabled class when not under /docs.

@nate-double-u
Copy link
Copy Markdown
Contributor Author

nate-double-u commented May 28, 2021

Thanks. Let's refactor this so that the dropdown html is always present, but the anchor has the added disabled class when not under /docs.

I'm not sure how we can (if I'm understanding this request correctly).

If you mean something like
<a class="nav-link dropdown-toggle {{ if ne .Section "docs" -}}disabled{{ end -}}" id="navbarDropdown" role="button" data-toggle="dropdown" aria-haspopup="true" aria-expanded="false">
instead of the if else structure I've put in around the anchor/dropdown, then the template fails because the dropdown menu is looking for front-matter in the docs files that isn't present in the non-versioned pages.

I'm not sure what dropdown html should be built on non versioned pages.

@chalin
Copy link
Copy Markdown
Contributor

chalin commented May 28, 2021

If you mean something like

Yes, almost: move the space inside the if: <a class="nav-link dropdown-toggle{{ if ne .Section "docs" -}} disabled{{ end -}}"

the template fails

Darn. I wasn't expecting useful dropdown entires, but also wasn't expecting it to fail.

Ok then, put the if eq .Section "docs" around the content of the <div class="dropdown-menu" ...></div>. Thanks.

@nate-double-u nate-double-u force-pushed the 304-versions-menu-ux-update branch from 249fdb0 to bd6b81b Compare May 28, 2021 20:12
@nate-double-u
Copy link
Copy Markdown
Contributor Author

nate-double-u commented May 28, 2021

Ok then, put the if eq .Section "docs" around the content of the <div class="dropdown-menu" ...></div>. Thanks.

The last (force pushed) commit does this -- but I don't like having an empty dropdown as much as having a visually distinct disabled versions selector.

Copy link
Copy Markdown
Contributor

@chalin chalin left a comment

Choose a reason for hiding this comment

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

I don't like having an empty dropdown as much as having a visually distinct disabled versions selector.

Right, neither do I. We need both. So you can add the conditional inclusion of disabled (the way you had suggested earlier).

@nate-double-u
Copy link
Copy Markdown
Contributor Author

... We need both. So you can add the conditional inclusion of disabled (the way you had suggested earlier).

OK, added -- on Firefox disabling doesn't allow the dropdown to work, so we don't get an empty one on click. waiting for the build to check other browsers.

@nate-double-u
Copy link
Copy Markdown
Contributor Author

wait, i didn't do that quite correctly.
give me a moment :)

@nate-double-u
Copy link
Copy Markdown
Contributor Author

OK, the latest commit gives the dropdown-menu a chance to be empty if it's able to be shown at all.

@nate-double-u nate-double-u requested a review from chalin May 28, 2021 20:36
@chalin chalin force-pushed the 304-versions-menu-ux-update branch from 1acf569 to 6d4bb2d Compare June 1, 2021 13:56
@chalin chalin changed the title Updating navbar-version-selector.html Always show Versions menu, but disabled when necessary Jun 1, 2021
@chalin
Copy link
Copy Markdown
Contributor

chalin commented Jun 1, 2021

@nate-double-u - On Friday I was on the fence between accepting your original change and asking for extra rework in the context of this PR or later. I've changed my mind and, for now at least, I prefer the simplicity of your original change -- so I went ahead and stripped the extra commits to leave only the first one. While there is an opportunity to rework this fix to keep things a bit more DRY, that can come later.

@chalin chalin changed the title Always show Versions menu, but disabled when necessary Always show Versions menu, though disabled when necessary Jun 1, 2021
@chalin chalin merged commit 0668c0c into main Jun 1, 2021
@chalin chalin deleted the 304-versions-menu-ux-update branch June 1, 2021 14:03
@chalin chalin mentioned this pull request Jun 1, 2021
6 tasks
NotAwar pushed a commit to NotAwar/website that referenced this pull request Feb 27, 2025
Now we always display the Versions menu but have made it inactive on pages where it doesn't apply

Signed-off-by: Nate W <4453979+nate-double-u@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Versions menu UX issue

2 participants