feat(ui): premium app features — installability, WCO, share, badging#2982
feat(ui): premium app features — installability, WCO, share, badging#2982tomayac wants to merge 25 commits into
Conversation
Enable the service worker (previously disabled), configure Workbox with network-only navigation fallback to preserve ISR freshness, and add standalone display mode to the manifest so browsers offer installation. Add a PwaPrompt component that surfaces a toast when a new SW version is waiting, and an install button in settings that appears only when the browser fires beforeinstallprompt.
Adds a Share button to the package header button group (alongside Compare and Likes) that invokes navigator.share with the package name, description, and current URL. The button only renders when the browser supports the Web Share API and is also registered as a command palette action.
Enable display_override: window-controls-overlay so the app header fills the installed-PWA title bar. The header background becomes the drag handle; the nav is explicitly non-draggable. env(titlebar-area-y/x) push content below the OS controls and clear the macOS traffic lights. Add three manifest shortcuts (Search, Compare, Settings) so users can jump straight to key destinations from the home-screen/taskbar icon.
Use input-switch-polyfill so Safari renders a native OS switch control and other browsers get a consistent polyfilled fallback. The elaborate hand-painted CSS is replaced by a single accent-color custom property that integrates with the user's chosen accent. Polyfill JS is loaded lazily only when 'switch' is absent from HTMLInputElement.prototype.
useAppBadge wraps navigator.setAppBadge/clearAppBadge with feature detection. useLikesBadge activates when the npm connector is connected: it loads the user's packages once, then polls /api/social/likes/:pkg every 10 minutes and sets the badge to the count of new likes received since the previous check. The baseline is persisted in localStorage per npm username so the badge never fires on the very first visit.
Adds a Playwright-based script (scripts/generate-pwa-screenshots.ts) that captures the homepage and a package page in light + dark mode at desktop (1280×800) and mobile (390×844) viewports. The script auto-starts nuxt preview, takes all 8 screenshots, then stops the server. Pass --url <url> to skip the local server and screenshot any running instance (useful in CI: --url https://npmx.dev). The PWA manifest now includes a screenshots[] array (nuxt.config.ts) with form_factor 'wide'/'narrow' entries, enabling Chrome's richer install dialog on desktop (Chrome 108+) and Android (Chrome 94+). Usage: pnpm build && pnpm generate:screenshots # commit public/screenshots/*.png
Add id: '/' to the web app manifest for stable PWA identity across manifest URL changes (spec-recommended practice). Generate all 8 PWA screenshots (desktop/mobile × dark/light × home/package) and commit them so Vercel CI picks them up. Chrome 108+ on desktop will show these in the richer install dialog carousel.
Window Controls Overlay:
- Fix header and scroll container being 15 px short of right edge by
resetting scrollbar-gutter to auto in WCO mode (html { scrollbar-gutter:
stable } reserved a gutter even when overflow: hidden removed the bar)
- Make header position:fixed and span full viewport width (inset-inline: 0)
in WCO mode so its border-bottom reaches the window edge
- Add #app-scroll fixed scroll container starting at the header's bottom
border so the scrollbar track never appears in the title bar
- Solid opaque header background (--bg) and no backdrop-filter in WCO mode
- Full-width nav with two-value padding-inline to handle both macOS traffic
lights (left) and Windows min/max/close (right)
- All interactive descendants declare no-drag; empty header space is drag
- Keep theme-color meta in sync with --bg (oklch) by writing directly to the
DOM node and guarding against @unhead re-asserting the PWA module's static
value after onMounted
Share button:
- Register 'v' keyboard shortcut; programmatic click anchors the share sheet
at the button position instead of the mouse cursor
- Include og:image as a shareable file when the browser supports file sharing
- Expose click() on ButtonBase via defineExpose for programmatic triggering
input-switch-polyfill:
- Bump to 1.12.0
- Add MutationObserver that re-syncs --switch-accent on every color-mode or
accent-color change so switches react live without a page reload
Adds an ambient module declaration for the untyped input-switch-polyfill package and fixes a noUncheckedIndexedAccess violation when reading the first matched switch element.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds PWA install/update support and manifest updates, package sharing, badge handling, window-controls-overlay shell behaviour, and switch polyfill support, with matching locale, schema, dependency, and screenshot tooling changes. ChangesPWA, sharing, badges, and shell support
Sequence Diagram(s)sequenceDiagram
participant ServiceWorker
participant PwaPrompt
participant SettingsPage
participant User
ServiceWorker->>PwaPrompt: needRefresh
PwaPrompt->>User: show update toast
User->>PwaPrompt: updateServiceWorker()
User->>SettingsPage: open install section
SettingsPage->>User: show install button
User->>SettingsPage: $pwa.install()
sequenceDiagram
participant User
participant PackageHeader
participant ShareButton
participant Navigator
User->>PackageHeader: choose share command or button
PackageHeader->>ShareButton: trigger sharePackage()
ShareButton->>ShareButton: read og:image and build ShareData
ShareButton->>Navigator: navigator.share(data)
Navigator-->>User: native share sheet
sequenceDiagram
participant LikesBadgePlugin
participant useLikesBadge
participant API
participant LocalStorage
participant useAppBadge
LikesBadgePlugin->>useLikesBadge: initialise
useLikesBadge->>API: list likes for cached packages
API-->>useLikesBadge: counts
useLikesBadge->>LocalStorage: loadStored / saveStored
useLikesBadge->>useAppBadge: setBadge() or clearBadge()
sequenceDiagram
participant BrowserChrome
participant AppShell
participant AppHeader
participant AppScroll
BrowserChrome->>AppShell: display-mode: window-controls-overlay
AppShell->>AppHeader: fixed top header styles
AppShell->>AppScroll: fixed internal scroll region
AppShell->>BrowserChrome: theme-color meta updated from --bg
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hello! Thank you for opening your first PR to npmx, @tomayac! 🚀 Here’s what will happen next:
|
This comment was marked as resolved.
This comment was marked as resolved.
…-features # Conflicts: # pnpm-lock.yaml
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
app/composables/useLikesBadge.ts (1)
54-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNon-null assertion on array index bypasses the strict index-safety guideline.
userPackages.value[i]!at Line 58 uses a non-null assertion instead of checking the value before use. As per coding guidelines, "Ensure you write strictly type-safe code, for example by ensuring you always check when accessing an array value by index." Prefer destructuring the package name alongside the result to avoid re-indexing.♻️ Proposed fix
const current: Record<string, number> = {} - for (let i = 0; i < userPackages.value.length; i++) { - const r = results[i] - if (r?.status === 'fulfilled') { - current[userPackages.value[i]!] = r.value.totalLikes - } - } + userPackages.value.forEach((pkg, i) => { + const r = results[i] + if (r?.status === 'fulfilled') { + current[pkg] = r.value.totalLikes + } + })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/composables/useLikesBadge.ts` around lines 54 - 60, The loop in useLikesBadge is re-indexing userPackages.value and using a non-null assertion on the package name, which bypasses strict index safety. Update the logic in the current/results processing to avoid userPackages.value[i]! by destructuring or otherwise carrying the package name together with the corresponding promise result before awaiting, then use that safe value when populating current.Source: Coding guidelines
app/app.vue (1)
248-280: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHeader-height magic number duplicated across files.
calc(env(titlebar-area-y, 0px) + 3.5rem + 1px)(Line 275) must stay in sync withAppHeader.vue'snavheight (min-h-14= 3.5rem) plus its border-bottom. Any future change to the header's height inAppHeader.vuewill silently desync this offset, causing the scrollbar/content to start at the wrong position in WCO mode.Consider exposing the header height as a shared CSS custom property (e.g. set on
:rootfromAppHeader.vue, consumed here) to keep the two files in sync.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/app.vue` around lines 248 - 280, The WCO scroll offset in App.vue is duplicating AppHeader.vue’s header height, so update the fixed `#app-scroll` positioning to derive its top offset from a shared CSS custom property instead of hardcoding 3.5rem + 1px. Set that custom property from AppHeader.vue based on the nav’s actual height and border, then consume it in the `@media` (display-mode: window-controls-overlay) block so both files stay in sync if the header changes.app/components/AppHeader.vue (1)
260-263: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInteractive-element selector may miss some focusable/clickable elements.
The
:is(a, button, input, select, [role='button'], [role='combobox'])list doesn't covertextarea,[role='menuitem']/[role='tab'], or arbitrary elements with atabindexand click handler that some components (e.g.LogoContextMenu) might render. Any such element left with inheriteddragwould swallow clicks under WCO.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/components/AppHeader.vue` around lines 260 - 263, The interactive-element selector in AppHeader’s header drag override is too narrow and can leave focusable/clickable elements inheriting drag behavior. Update the selector in the `header :deep(:is(...))` rule to also cover missing interactive cases such as `textarea`, `[role='menuitem']`, `[role='tab']`, and generic tabbable/clickable elements (for example those with `tabindex`), so components like `LogoContextMenu` don’t have clicks swallowed under WCO.test/unit/a11y-component-coverage.spec.ts (1)
64-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPlease test
PwaPrompt.client.vueinstead of skipping it.This component does not need a real service worker to render; a vitest mount can stub
useNuxtApp().$pwa.needRefreshand exercise the new alert and action buttons. Keeping it on the skip list leaves the update prompt without any accessibility coverage. As per coding guidelines,**/*.{test,spec}.{ts,tsx}: "Write unit tests for core functionality usingvitest".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/unit/a11y-component-coverage.spec.ts` at line 64, The a11y coverage list is incorrectly skipping PwaPrompt.client.vue, so it never gets tested. Remove the skip entry from the coverage spec and add a vitest mount-based test for PwaPrompt.client.vue that stubs useNuxtApp().$pwa.needRefresh to render the prompt and verify the alert plus action buttons. Locate the change in the a11y-component-coverage.spec.ts entry for PwaPrompt.client.vue and update the related component test coverage accordingly.Source: Coding guidelines
app/components/Package/Header.vue (1)
81-87: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShare through one code path.
This duplicates the payload building in
app/components/Package/ShareButton.client.vuebut omits the optionalog:imagefile, so the command-palette action already behaves differently from the visible share button. Please extract a shared helper/composable and reuse it from both entry points.Also applies to: 114-123
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/components/Package/Header.vue` around lines 81 - 87, The package sharing logic is duplicated between Header.vue’s sharePackage and ShareButton.client.vue, and the Header path currently omits the optional og:image file, causing behavior drift. Extract the payload-building and navigator.share invocation into a shared helper/composable, then reuse it from both share entry points so both paths include the same fields and fallbacks, including the og:image file when available.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/components/Package/ShareButton.client.vue`:
- Around line 45-50: The share flow in ShareButton.client.vue is setting
shareData.files after getOgImageFile() returns, but the real payload is not
being validated before navigator.share is called. Update the ShareButton logic
around the shareData object so the final ShareData shape is checked with files
included before mutating shareData, using getOgImageFile() and the
navigator.share call as the key points to adjust. Keep the URL/text fallback
available by only attaching the file once the full payload is confirmed valid.
In `@app/composables/useLikesBadge.ts`:
- Around line 25-43: The `useLikesBadge` logic has a race between the `npmUser`
refresh watcher and the immediate `checkLikes()` watcher, which can persist a
stale package baseline for a new user. Update `useLikesBadge` so `checkLikes()`
only runs after the package list refresh for the current `npmUser` has
completed, either by sharing the refresh path in the `npmUser` watcher or by
introducing a readiness guard that blocks the immediate watcher until
`userPackages` is current. Make the fix around the `watch(npmUser, ...)` block
and the `checkLikes()` watcher so the first poll always uses the new user’s
package list.
In `@scripts/generate-pwa-screenshots.ts`:
- Around line 42-44: The `generate-pwa-screenshots.ts` argument parsing for
`--url` does not validate that a value follows the flag before reading
`args[urlFlagIdx + 1]`. Update the `explicitUrl` handling to first confirm
`--url` is present and that the next array entry exists and is a valid non-empty
string; if it is missing, fail fast with a clear error instead of falling back
to the preview-server path. Keep the fix localized to the
`args`/`urlFlagIdx`/`explicitUrl` logic so the script’s existing flow remains
unchanged.
- Around line 96-109: The startup flow in startPreviewServer currently leaves
the spawned nuxt preview running if waitForServer(url) fails, and
server.on('error') throws outside main().catch(). Change the preview startup to
reject/propagate errors through the Promise returned by startPreviewServer, and
make sure the child process is terminated/cleaned up before rejecting or
rethrowing. Use the existing startPreviewServer, waitForServer, and server event
handlers as the main places to fix this.
---
Nitpick comments:
In `@app/app.vue`:
- Around line 248-280: The WCO scroll offset in App.vue is duplicating
AppHeader.vue’s header height, so update the fixed `#app-scroll` positioning to
derive its top offset from a shared CSS custom property instead of hardcoding
3.5rem + 1px. Set that custom property from AppHeader.vue based on the nav’s
actual height and border, then consume it in the `@media` (display-mode:
window-controls-overlay) block so both files stay in sync if the header changes.
In `@app/components/AppHeader.vue`:
- Around line 260-263: The interactive-element selector in AppHeader’s header
drag override is too narrow and can leave focusable/clickable elements
inheriting drag behavior. Update the selector in the `header :deep(:is(...))`
rule to also cover missing interactive cases such as `textarea`,
`[role='menuitem']`, `[role='tab']`, and generic tabbable/clickable elements
(for example those with `tabindex`), so components like `LogoContextMenu` don’t
have clicks swallowed under WCO.
In `@app/components/Package/Header.vue`:
- Around line 81-87: The package sharing logic is duplicated between
Header.vue’s sharePackage and ShareButton.client.vue, and the Header path
currently omits the optional og:image file, causing behavior drift. Extract the
payload-building and navigator.share invocation into a shared helper/composable,
then reuse it from both share entry points so both paths include the same fields
and fallbacks, including the og:image file when available.
In `@app/composables/useLikesBadge.ts`:
- Around line 54-60: The loop in useLikesBadge is re-indexing userPackages.value
and using a non-null assertion on the package name, which bypasses strict index
safety. Update the logic in the current/results processing to avoid
userPackages.value[i]! by destructuring or otherwise carrying the package name
together with the corresponding promise result before awaiting, then use that
safe value when populating current.
In `@test/unit/a11y-component-coverage.spec.ts`:
- Line 64: The a11y coverage list is incorrectly skipping PwaPrompt.client.vue,
so it never gets tested. Remove the skip entry from the coverage spec and add a
vitest mount-based test for PwaPrompt.client.vue that stubs
useNuxtApp().$pwa.needRefresh to render the prompt and verify the alert plus
action buttons. Locate the change in the a11y-component-coverage.spec.ts entry
for PwaPrompt.client.vue and update the related component test coverage
accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cc5c172b-099f-4e84-8a6c-497d259c7474
⛔ Files ignored due to path filters (9)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlpublic/screenshots/desktop-dark-home.pngis excluded by!**/*.pngpublic/screenshots/desktop-dark-package.pngis excluded by!**/*.pngpublic/screenshots/desktop-light-home.pngis excluded by!**/*.pngpublic/screenshots/desktop-light-package.pngis excluded by!**/*.pngpublic/screenshots/mobile-dark-home.pngis excluded by!**/*.pngpublic/screenshots/mobile-dark-package.pngis excluded by!**/*.pngpublic/screenshots/mobile-light-home.pngis excluded by!**/*.pngpublic/screenshots/mobile-light-package.pngis excluded by!**/*.png
📒 Files selected for processing (18)
app/app.vueapp/components/AppHeader.vueapp/components/Button/Base.vueapp/components/Package/Header.vueapp/components/Package/ShareButton.client.vueapp/components/PwaPrompt.client.vueapp/components/Settings/Toggle.client.vueapp/composables/useAppBadge.tsapp/composables/useLikesBadge.tsapp/pages/settings.vueapp/plugins/input-switch-polyfill.client.tsapp/plugins/likes-badge.client.tsapp/types/input-switch-polyfill.d.tsi18n/locales/en.jsonnuxt.config.tspackage.jsonscripts/generate-pwa-screenshots.tstest/unit/a11y-component-coverage.spec.ts
This comment was marked as resolved.
This comment was marked as resolved.
navigator.canShare() was only checked against a dummy file to test generic files-sharing support, not against the actual title/text/url/files combination. Some implementations support sharing files or url/text but not both together, so attaching the file unconditionally could make navigator.share() reject silently. Now the file is only attached once canShare() confirms the full payload is shareable.
Previously a missing value after --url silently fell through to the preview-server path instead of erroring, which is confusing when the flag was clearly intended to be used.
gameroman
left a comment
There was a problem hiding this comment.
This probably needs a discussion first
(I was chatting about this with @danielroe.) |
…to pwa-and-premium-app-features
- Drop the standalone input-switch-polyfill.d.ts ambient module file (knip flagged it as an unused file) in favor of a targeted @ts-expect-error at the one dynamic-import call site. - Regenerate i18n/schema.json to include the new pwa.*, settings.app, package.links.share and package.share_aria_label keys. - Pin vue to 3.5.39 in pnpm-workspace.yaml overrides; the regenerated lockfile had resolved two separate vue versions (3.5.34 and 3.5.39).
Package sub-headers use sticky top-14 to clear the fixed <header> when the viewport is the scroll container. In WCO mode #app-scroll already starts below the header, so that offset left a redundant 3.5rem gap between the title bar and the sticky sub-header instead of gluing it to the header.
@vite-pwa/nuxt's client plugin only registers a beforeinstallprompt listener when pwa.client.installPrompt is truthy; it defaults to false. Without it, showInstallPrompt never flips to true and the Settings "Install app" button never appears, even though the browser fires the event normally.
ghostdevv
left a comment
There was a problem hiding this comment.
There are a couple distinct, but related, changes in this PR - could you split them up into multiple PRs?
|
|
||
| defineExpose({ | ||
| focus: () => el.value?.focus(), | ||
| click: () => el.value?.click(), |
There was a problem hiding this comment.
iirc vue would auto expose this
There was a problem hiding this comment.
Verified against Vue's own source (@vue/runtime-core, getComponentPublicInstance): once a component calls defineExpose(), the proxy a parent's template ref sees only contains what's explicitly listed, plus Vue's $-prefixed magic properties ($el, $props, $emit, etc., from publicPropertiesMap) — there's no fallback to arbitrary DOM methods like .click().
// @vue/runtime-core
function getComponentPublicInstance(instance) {
if (instance.exposed) {
return instance.exposeProxy || (instance.exposeProxy = new Proxy(proxyRefs(markRaw(instance.exposed)), {
get(target, key) {
if (key in target) return target[key]
else if (key in publicPropertiesMap) return publicPropertiesMap[key](instance)
},
...So without this line, buttonRef.value?.click() in ShareButton.client.vue (used for the v keyboard shortcut, so the share sheet anchors at the button rather than the cursor) would just be undefined. It's consistent with the existing focus/getBoundingClientRect entries above it, which already manually forward DOM methods the same way rather than relying on $el — keeping click explicit here matches that pattern. Happy to switch to buttonRef.value?.$el?.click() at the call site instead if you'd prefer avoiding the extra expose entry, though that trades one explicit line for a slightly fuzzier $el cast.
| sharp: 0.34.5 | ||
| vite: npm:@voidzero-dev/vite-plus-core@0.1.20 | ||
| vitest: npm:@voidzero-dev/vite-plus-test@0.1.20 | ||
| vue: 3.5.39 |
There was a problem hiding this comment.
Resolving a pnpm-lock.yaml merge conflict caused pnpm to resolve two different vue versions (3.5.34 via @nuxt/test-utils, 3.5.39 elsewhere), despite dedupePeers already being set. Two Vue instances in one app is a real hazard since reactivity tracking is per-instance. Pinning forces a single resolved copy.
| @@ -0,0 +1,26 @@ | |||
| export default defineNuxtPlugin(async () => { | |||
There was a problem hiding this comment.
We already have switches styled with role="switch" - what's the benefit of polyfiling this?
There was a problem hiding this comment.
The reason to use the switch attribute with the polyfill is that on WebKit you get the real switch control (and the polyfill isn't even loaded), and the polyfill supports all the features the real control supports, like the accent color and also dragging the thumb to change the state of the toggle, which CSS-based solutions don't support.
Drops the main() wrapper in favor of top-level await now that the project runs as ESM, per review feedback.
Not a convention used elsewhere in the repo.
…ate PRs Per review feedback, keeps this PR focused on PWA installability, WCO, and app badging. The Share button and native switch polyfill now live in npmx-dev#2997 and npmx-dev#2998 respectively.
|
Per the review feedback about scope, I've split this PR:
This PR is now focused on PWA installability, WCO, and app badging. |
…-features # Conflicts: # pnpm-lock.yaml
app/plugins/likes-badge.client.ts registered useLikesBadge() as a global Nuxt plugin, which runs for every mountSuspended() call in component tests — including standalone tests that mock useConnector for just the component under test. That caused useLikesBadge()'s own useConnector() call to resolve the real, createSharedComposable-backed implementation instead of the mock, which HeaderConnectorModal.spec.ts and use-command-palette-commands.spec.ts's assertions depend on. Moving the call into app.vue keeps it always active in the real app (app.vue is always mounted there) while no longer running during tests that mount an unrelated component in isolation.
Adds settings.connector.showLikesBadge (default on), surfaced as a toggle in the connector modal's connected-state preferences — so it's only shown once connected, alongside the existing auto-open-url toggle. useLikesBadge() now skips fetching, badging, and polling entirely while the setting is off, clearing any existing badge immediately when toggled off.






Summary
@vite-pwa/nuxt) with a manifest id and generated install-UI screenshots.<input type="checkbox" switch>polyfill, kept live-synced with the accent color/theme.theme-colorin sync with--bgso the WCO title-bar strip matches the header.Test plan
pnpm test:unit— 1652/1652 passingpnpm run test:types— passing (fixed 2 pre-existing type errors in the switch polyfill plugin)vp lint— 0 errors (3 pre-existing style warnings, unrelated to correctness)pnpm run build— succeeds