-
-
Notifications
You must be signed in to change notification settings - Fork 119
feat: add package manager select dropdown #379
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
|
||
| <template> | ||
| <div class="relative"> | ||
| <button |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Chose to build a custom dropdown because I think icons don't show up well in native select. Someone please correct me if I'm wrong!
app/components/ConnectorModal.vue
Outdated
| aria-hidden="true" | ||
| /> | ||
| </button> | ||
| <div class="ml-auto flex items-center gap-2"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use ms-auto instead ml-auto (RTL support): check the contributing guide: https://github.com/npmx-dev/npmx.dev/blob/main/CONTRIBUTING.md#rtl-support
| </template> | ||
| </ClientOnly> | ||
| <span | ||
| class="i-carbon-chevron-down w-3 h-3" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use : to split collection and icon (small perf at UnoCSS)
|
much nicer 👌 |
| : undefined | ||
| " | ||
| :aria-label="$t('settings.package_manager')" | ||
| class="absolute right-0 top-full mt-1 min-w-28 bg-bg-elevated border border-border rounded-lg shadow-lg z-50 overflow-hidden py-1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't use left/right use inset-is and inset-ie respectivelly (when using absolute psotioning)
| <span>{{ pm.label }}</span> | ||
| <span | ||
| v-if="selectedPM === pm.id" | ||
| class="i-carbon-checkmark w-3 h-3 text-accent ml-auto shrink-0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replace ml-auto with ms-auto
|
FYI: #393 might mean this needs to have conflicts resolved. we wouldn't render the tabs any more of course, but we'd keep the same approach for rendered install commands |
6b62ba1 to
c82d65d
Compare
app/pages/[...package].vue
Outdated
| <PackageManagerTabs /> | ||
| <PackageManagerSelect /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: Remove PackageManagerTabs
c82d65d to
eac64c4
Compare
This replaces the tab list so it's extra clear to the user what package manager they've selected. It also lets use use it in the connector modal to be explicit on the package manager choice.
eac64c4 to
41bd452
Compare
| /> | ||
| </button> | ||
| <div class="ms-auto flex items-center gap-2"> | ||
| <PackageManagerSelect /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy to remove this if we think it's too crowded in the connector install area.
| </button> | ||
|
|
||
| <!-- Dropdown menu (teleported to body to avoid clipping) --> | ||
| <Teleport to="body"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't think of a better way to avoid clipping. But this means if JS is disabled you won't be able to open the dropdown. I checked in prod and currently you can't interact with the tabs with JS disabled.
It's not great either way, let me know if this is a blocker!
| :aria-label="$t('package.get_started.pm_label')" | ||
| @keydown="onTabListKeydown" | ||
| > | ||
| <button |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the way this works right now is that we render all the tabs + all the commands, and we use a data attribute on the root of the document to determine which one is displayed.
this means there's no flicker if you hard reload.
see #393
would you be able to follow the same pattern in this one? (or if it's tricky, let me know and I can adapt the same thing I did there, here....)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I just noticed that! Yes let me copy that pattern across.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me know what you think of the updates. Thanks again for reminding me about that!
| <span | ||
| class="pm-select-content" | ||
| :data-pm-select="pmOption.id" | ||
| :aria-hidden="pmOption.id !== selectedPM" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not ideal that we generate DOM elements that aren't visible. I tried to mitigate it for screen readers with aria hidden, but I'm open to ideas!
|
Sorry @danielroe I missed the e2e test! |
danielroe
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
beautiful!

This replaces the tab list so it's extra clear to the user what package manager they've selected. It also lets use use it in the connector modal to be explicit on the package manager choice.
The dropdown is inspired by Stripe's dev docs
Testing
✅ Tests pass locally
Light Mode Desktop
light-mode-package-desktop.mp4
light-mode-connector-modal.mp4
Dark Mode Desktop
dark-mode-connector-modal.mp4
dark-mode-package-desktop.mp4
Mobile view
mobile-dropdown.mp4
Small bugfix on focus ring being cut off
hard-refresh-test.mp4