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.

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

feat: Persist user's package manager choice #1181

Closed
wants to merge 0 commits into from

Conversation

Michael-Obele
Copy link
Contributor

Addressing issue #1059: Persistent Package Manager Selection

This PR enhances user experience by introducing a persistent package manager selection feature. The user's chosen package manager is now saved and automatically used across pages within a session, eliminating the need for repeated prompts.

Key Improvements:

  • Persistent storage: User preferences are stored using a writable store for easy retrieval.
  • UI enhancements: The selected package manager is clearly displayed with an option to change it.
  • Improved user experience: Reduces user friction by eliminating repetitive prompts.

Changes:

  • Integrated selectedCommand writable store to manage user preferences.
  • Updated copy-button.svelte to use the selected package manager.
  • Enhanced UI to display and allow modification of the package manager.

Note: The selected package manager is currently reset on page reload.

Showcase

Screencast.from.2024-07-12.05-01-54.webm

Copy link

changeset-bot bot commented Jul 12, 2024

⚠️ No Changeset found

Latest commit: 2e6474a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

github-actions bot commented Jul 12, 2024

built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name Status Preview Last Commit
shadcn-svelte ✅ Ready (View Log) Visit Preview f5f57e4

@huntabyte
Copy link
Owner

huntabyte commented Jul 13, 2024

Hey @Michael-Obele ! Thanks for taking this on! I'll give it a review tomorrow! One thing I do see is that the bun lockfile will need to be removed before merge, as we use pnpm 😃

@huntabyte
Copy link
Owner

huntabyte commented Jul 15, 2024

Hey @Michael-Obele, the functionality is great. I'm not sure how I feel about having the package manager's name show up beside the icon like that, though. It seems a bit unintuitive. Perhaps we can set it in the CMD+K menu (similar to themes), where we have an option like "Set Default Package Manager" or "Clear Default Package Manager" for power users.

If they have it set, it just copies the command for their specific PM. Otherwise, it shows the dropdown for the non-power users. What do you think about that?

@tryggs
Copy link

tryggs commented Jul 15, 2024

Good job!

As @huntabyte said, the functionality is there but for me at least, I feel like its a bit unclear what is going on. Now the copy button still prompts the user to pick libary, and that the text (bun, npm, etc) copies the selected value.

How would a dropdown next too the copy button look? The value of the dropdown would be saved in local storage.

See Stripes codeblock in their docs.

@huntabyte
Copy link
Owner

Good job!

As @huntabyte said, the functionality is there but for me at least, I feel like its a bit unclear what is going on. Now the copy button still prompts the user to pick libary, and that the text (bun, npm, etc) copies the selected value.

How would a dropdown next too the copy button look? The value of the dropdown would be saved in local storage.

See Stripes codeblock in their docs.

Oh I quite like this idea as well!

@Michael-Obele
Copy link
Contributor Author

Hey @Michael-Obele, the functionality is great. I'm not sure how I feel about having the package manager's name show up beside the icon like that, though. It seems a bit unintuitive. Perhaps we can set it in the CMD+K menu (similar to themes), where we have an option like "Set Default Package Manager" or "Clear Default Package Manager" for power users.

If they have it set, it just copies the command for their specific PM. Otherwise, it shows the dropdown for the non-power users. What do you think about that?

This would be nice, but I can see how people would easily get confused by the options of set and clear, I would suggest, however, that if we want to go with this design, we use a single click for the default (which would be whatever was last used) and a right click/context menu to select a new default. I use multiple package managers so I tend to switch often, so I hope to make the switch as easy as possible.

@Michael-Obele
Copy link
Contributor Author

Hey @Michael-Obele ! Thanks for taking this on! I'll give it a review tomorrow! One thing I do see is that the bun lockfile will need to be removed before merge, as we use pnpm 😃

Got it, I'll do that. 😊

@Michael-Obele
Copy link
Contributor Author

Good job!

As @huntabyte said, the functionality is there but for me at least, I feel like its a bit unclear what is going on. Now the copy button still prompts the user to pick libary, and that the text (bun, npm, etc) copies the selected value.

How would a dropdown next too the copy button look? The value of the dropdown would be saved in local storage.

See Stripes codeblock in their docs.

I think this design is the best because it is simple to use, requires less pasting from an unknown manager because you can always tell which manager you are on. The local stage would probably be required.
If we're okay with the design, I can implement it, and we can see how it looks.

@tryggs
Copy link

tryggs commented Jul 16, 2024

Good job!
As @huntabyte said, the functionality is there but for me at least, I feel like its a bit unclear what is going on. Now the copy button still prompts the user to pick libary, and that the text (bun, npm, etc) copies the selected value.
How would a dropdown next too the copy button look? The value of the dropdown would be saved in local storage.
See Stripes codeblock in their docs.

I think this design is the best because it is simple to use, requires less pasting from an unknown manager because you can always tell which manager you are on. The local stage would probably be required. If we're okay with the design, I can implement it, and we can see how it looks.

Me, personally, do think that it would be the best alternative, since it makes it very clear what you are copying, while always being one click away. Make sure that the content of the code block is the correct one for the selected value 😄

@Michael-Obele
Copy link
Contributor Author

Good job!
As @huntabyte said, the functionality is there but for me at least, I feel like its a bit unclear what is going on. Now the copy button still prompts the user to pick libary, and that the text (bun, npm, etc) copies the selected value.
How would a dropdown next too the copy button look? The value of the dropdown would be saved in local storage.
See Stripes codeblock in their docs.

I think this design is the best because it is simple to use, requires less pasting from an unknown manager because you can always tell which manager you are on. The local stage would probably be required. If we're okay with the design, I can implement it, and we can see how it looks.

Me, personally, do think that it would be the best alternative, since it makes it very clear what you are copying, while always being one click away. Make sure that the content of the code block is the correct one for the selected value 😄

Yes, I think it is great, but it might not be so simple to display the right text depending on the package manager. Essentially, the copy function simply modifies the necessary portions of the text (such as the npx to bunx) because the code is hardcoded in a markdown format. We should definitely ask @huntabyte. I could simply write a function to change the display, but I am not sure if that would be necessary.

@tryggs
Copy link

tryggs commented Aug 23, 2024

How's it going?

I decided today that wanted to learn how to contribute to open-source projects (something I've never done before), and since I haven't seen this being implemented yet, I was thinking maybe I could give it a go.

@Michael-Obele
Copy link
Contributor Author

Hey @tryggs
Though I have not heard back regarding the suggested implementation, I am still working on this.
I have already implemented the updated design, which resembles the Stripes codeblock. If everything is okay, I will update the PR and see if this can be merged shortly. I would really like to see this issue fixed.

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.

3 participants