Skip to content

Sidebar state fix #106

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Sidebar state fix #106

wants to merge 2 commits into from

Conversation

firebed
Copy link

@firebed firebed commented Apr 13, 2025

In shadcn-vue docs they mention that SIDEBAR_COOKIE_NAME is sidebar_state but in the actual code in sidebar/utils.ts it's sidebar:state (with semicolon)

Without this fix, the sidebar looses its state everytime the user navigates to a different page or refreshes the page.

@tnylea
Copy link
Contributor

tnylea commented Apr 14, 2025

Thanks! Good catch @firebed 💪

@tnylea tnylea added the Approved Approved for merge label Apr 14, 2025
@muhammadcahya
Copy link

Hi @firebed @tnylea
I believe we should retain sidebar_state, but the issue lies in utils.ts, specifically with the SIDEBAR_COOKIE_NAME constant to sidebar_state.

The react version has already addressed this (merged in shadcn/react) and the same fix is relevant for Vue. (credit : @longzheng )

here the statement :

The current sidebar cookie name is not valid because it contains a colon, which is not permitted (but most browsers will accept).

A <cookie-name> can contain any US-ASCII characters except for: control characters ([ASCII](https://developer.mozilla.org/en-US/docs/Glossary/ASCII) characters 0 up to 31 and ASCII character 127) or separator characters (space, tab and the characters: ( ) < > @ , ; : \ " / [ ] ? = { })

This causes issues with Mock Service Worker and other libraries which validate cookie names based on very strict rules.

See original fixes shadcn-ui/ui#6391

@firebed
Copy link
Author

firebed commented Apr 21, 2025

Update cookie name in utils.ts as suggested by @muhammadcahya

@tnylea - I'll implement the suggested approach by updating the cookie name in the utils.ts file. Would you prefer I:

  1. Close this PR and create a new one, or
  2. Revert the previous commits within this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Approved for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants