Skip to content

Refactor/commands page sidebar breadcrumbs #299

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.

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

Merged

Conversation

phiro56
Copy link
Contributor

@phiro56 phiro56 commented Jul 7, 2025

Refactor/commands page sidebar breadcrumbs
- Implement sidebar search functionality for commands
- Add no results message for sidebar search

Screenshot 2025-07-07 at 5 53 00 PM Screenshot 2025-07-07 at 5 53 11 PM Screenshot 2025-07-07 at 5 53 21 PM

(EDIT: Is this what you're doing with #293 ?)

Copy link
Member

@stockholmux stockholmux left a comment

Choose a reason for hiding this comment

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

Would it be possible to have the split background colours extend to left and right? That way we get the readable width without the abrupt white background on left/right. What do you think @madolson?

See mockup:
mock-commands

Original:
Screenshot 2025-07-11 at 8 07 49 AM

phiro56 added 2 commits July 16, 2025 19:09
- Refactor the commands page to include breadcrumb navigation and sidebar.

Signed-off-by: Daniel Phillips <[email protected]>
- Implement sidebar search functionality for commands
- Add no results message for sidebar search

Signed-off-by: Daniel Phillips <[email protected]>
@phiro56 phiro56 force-pushed the refactor/commands-page-sidebar-breadcrumbs branch from 137be22 to 0474395 Compare July 17, 2025 01:11
@phiro56
Copy link
Contributor Author

phiro56 commented Jul 17, 2025

@stockholmux rebase with main fixed the expanded background

Screenshot 2025-07-16 at 7 13 07 PM

Copy link
Member

@stockholmux stockholmux left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@stockholmux stockholmux merged commit 6c6a018 into valkey-io:main Jul 25, 2025
1 check passed
stockholmux added a commit that referenced this pull request Aug 5, 2025
…sue (#320)

### Description

It looks like in #299 the sidebar code was copy/pasted from the command
section into the command page. Both pages uses the global variable named
`command_data_obj`, so when there wasn't a container explicitly from the
command entry, it just used the one from the page.

In this PR, I namespaced the variable name to fix the command names in
the list. Additionally, I removed an unused global that sorted/grouped
all the commands.

I've also noticed that the build times have shot up after #299 to around
50 seconds - which is bonkers. Given the repetitious nature of this page
element, this should be abstracted into a macro and/or cached during
build. At time of writing there is 416 pages rendered in this section,
each one has to open 416 + 4 (json files that index all the commands).
That means that generating this sidebar adds 174,720 JSON file opens and
parses. I will file a seperate issue to track.

### Issues Resolved

#319

### Check List
- [x] Commits are signed per the DCO using `--signoff`

By submitting this pull request, I confirm that my contribution is made
under the terms of the BSD-3-Clause License.

Signed-off-by: Kyle J. Davis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants