Skip to content

feat: site wallpaper preview and coin shop frontend - #215

Draft
16th-admin wants to merge 1 commit into
mainfrom
codex/site-wallpapers
Draft

feat: site wallpaper preview and coin shop frontend#215
16th-admin wants to merge 1 commit into
mainfrom
codex/site-wallpapers

Conversation

@16th-admin

Copy link
Copy Markdown
Collaborator

Summary

  • Add three built-in static CSS wallpapers, with no uploads or remote image URLs.
  • Preview across normal community pages, shared header/footer and login card; retain readable inputs and content cards.
  • Add an own-profile Shop entry with Products and My backpack sections; each wallpaper is priced at 1,000 existing community coins.
  • Display cached userInfo.Gold explicitly as a cached balance; unknown balance remains unknown.
  • Include cross-tab persistence, reset, error handling and local browser tests.

Draft / backend blockers

Purchasing is disabled. No debit, purchase request, delivery or ownership record is created. Backpack service availability is unknown rather than presented as an empty inventory.

Wallpaper use is development-only until the ownership service is integrated. Production selection fails closed. Preview does not grant ownership. Community authorization and wallet debit capability are required before any independent service can charge the existing balance. Atomic debit/grant, idempotent orders, duplicate ownership protection, authoritative inventory and equipment persistence remain backend work.

Shop copy is Chinese/English; other locales currently use English for shop strings. Authenticated pages and real devices still need acceptance testing.

Validation

  • 12 local Playwright checks passed across Chromium and narrow-screen WebKit, including no debit/no purchase requests during preview.
  • Type check and production build passed in the implementation turn; existing build warnings remain.
  • Real API home/discussion feeds and login wallpaper visually checked in browser.
  • i18n key check and git diff --check passed before submission.
  • GitHub CI pending.

Preview

Run npm run dev -- --host 127.0.0.1 --port 5176 --strictPort.
Open /#/shop or /#/ for normal-page wallpaper preview.
Run npx playwright test --config scripts/tests/wallpapers.local.config.ts --workers=2.
See WALLPAPERS.md for scope and integration requirements.

Independent of avatar-frame PR #214. No changes to that PR are included here.

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

🤖 Codex-Style Code Review by OpenCode (Vue Best Practice Enabled)

💡 Autonomous AI Reviewer inspecting git commit history and Vue code quality.

Now I have all the context needed. Let me write the review report.


Code Review: feat: add site wallpaper preview and coin shop frontend

PR: adafe6ee750608 | +667 / -6 across 21 files


📋 Summary

This PR introduces a browser-local wallpaper system with three built-in CSS gradient backgrounds, a /wallpapers selection page, a /shop coin-shop frontend (purchase disabled pending backend), and a dev-only WallpaperPicker in the shared header. The wallpaper is persisted via localStorage, syncs across tabs, and applies via a ::before pseudo-element on #app.


🔴 Blocking Issues

1. Module-level ref creates a singleton shared across all consumers — potential intent mismatch

src/services/wallpapers.ts:29-30

export const wallpaperId = ref<WallpaperId | null>(readWallpaper())
export const wallpaper = computed(() => WALLPAPERS.find((item) => item.id === wallpaperId.value))

These are module-scope reactive values. This works correctly as a global store in this codebase, but it's a Vue anti-pattern: module-level ref() / computed() outside a composable create implicit singletons that are hard to test and can cause subtle bugs if the module is ever imported in a non-browser context (SSR, worker, test). The readWallpaper() call at module evaluation also calls localStorage unconditionally (guarded by import.meta.env.DEV at runtime but still evaluated at import time).

Recommendation: Consider wrapping in a createWallpaperStore() composable or using reactive({}) / Pinia for testability. This is non-blocking for the current dev-only use but should be noted for when this feature goes to production and is test-covered.

2. readWallpaper() accesses storage.getObj which returns a StorageResult.value usage is confusing

src/services/wallpapers.ts:25

const value = storage.getObj('siteWallpaper').value

storage.getObj() returns { status, value }. The .value here accesses the value property of the plain object — but because Vue 3 reactivity is also accessed via .value on refs, this reads as potentially confusing to maintainers. It's technically correct but could be improved with destructuring: const { value: stored } = storage.getObj('siteWallpaper').


🟡 Important Issues

3. Wallpapers.vue and Shop.vue duplicate the same select() / preview() + setWallpaper + feedback pattern

src/views/Wallpapers.vue:56-63, src/views/Shop.vue:63-73

Both views contain nearly identical try/catch around setWallpaper with a feedback ref. This violates DRY and means any future error-handling change must be made in both places.

Suggestion: Extract into a shared composable (e.g., useWallpaperSelect() returning { feedback, select }).

4. loginModel.vue adds a backgroundImage style on <n-card> — no fallback handling for scoped styles

src/components/popup/loginModel.vue:4

<n-card class="login-surface" :style="{ backgroundImage: wallpaper?.background }">

The :style binding sets backgroundImage inline. When wallpaper is null, this produces style="background-image: undefined" — Vue's reactivity will render background-image: undefined as background-image: none (correct behavior), but the existing scoped CSS for .login-surface has no background definition, so this relies entirely on inline styles. If Naive UI's <n-card> ever applies its own background, the inline backgroundImage alone won't create a visible layer — it would need background-size: cover and potentially background-position.

Recommendation: Add background-size: cover; background-position: center; to .login-surface styles in loginModel.vue, or at minimum document this assumption.

5. --page-background: transparent is set via class toggle but pages hardcode fallback colors

src/App.vue:37, src/views/Settings.vue:178, src/views/Editor.vue:626

The CSS variable --page-background is set to transparent when has-wallpaper class is present, and falls back to specific colors (e.g., #f5f5f5, #f4f7fb) when it isn't. However, the Header and Footer use white as fallback. This means when a wallpaper IS active, the Header/Footer become transparent (showing the wallpaper behind), but other pages also become transparent. This is the intended design per WALLPAPERS.md, but the consistency is fragile — any new view that uses a hardcoded background color will break the visual contract.

Suggestion: Document this pattern more prominently or enforce a CSS convention (e.g., all full-page views must use var(--page-background) instead of hardcoded colors).

6. Shop.vue accesses storage.getObj('userInfo').value at component setup time without reactivity

src/views/Shop.vue:56

const account = storage.getObj('userInfo').value

This reads the user info once at component creation. If the user logs in after the Shop page loads (or in another tab), account won't update. The balance display will be stale. This is a local cache read (as documented), but it would be better to make it reactive using a ref + watch on the storage event, or at least provide a way to refresh.

7. i18n: shop and wallpapers keys are present in zh and en but use English placeholders in fr, ja, de

The fr.ts, ja.ts, and de.ts files have shop and wallpapers keys that are copied verbatim from the English translations (identical English strings). This is fine for an initial PR but should be tracked — shipping English strings to non-English users is a regression in localized UX.


🟢 Nit / Suggestions

8. WallpaperPicker.vue uses <select> — keyboard UX is acceptable but not custom

The <select> element is fine for accessibility (native form control). No issue here.

9. readWallpaper() returns null in production — wallpaper is effectively invisible

src/services/wallpapers.ts:23-24

function readWallpaper(): WallpaperId | null {
  if (!import.meta.env.DEV) return null

This means in production builds, wallpaper selection is always null regardless of what's in localStorage. This is the intended fail-closed behavior (no ownership service), but it means the WallpaperPicker component (which checks import.meta.env.DEV) and the Shop.vue preview button are both invisible in production. The has-wallpaper class will never be applied. This is correct but worth confirming it's the intended long-term behavior, not just a "we forgot to remove this guard."

10. Test coverage is good — 5 Playwright tests covering key flows

scripts/tests/wallpapers.spec.ts covers: shop display, purchase disabled, wallpaper switching, persistence, cross-tab sync, unknown ID fallback, and storage error handling. Well-structured with Chinese-language test names (matches project conventions).

11. WALLPAPERS.md is a thorough design doc

Good practice. The doc accurately describes the current state, edge cases, and future integration requirements.

12. CSS variable pattern for background is clean

The var(--page-background, white) pattern with isolation: isolate and ::before pseudo-element is a solid, performant approach for a site-wide background effect.


✅ Decision: Approve with minor suggestions

This is a well-scoped, well-tested PR that correctly implements a dev-only wallpaper preview system with proper fail-closed production behavior. The architecture is clean, accessibility is maintained, and cross-tab sync works via native StorageEvent. The main concerns are:

  1. Module-level reactive singletons (not blocking now, but track for production)
  2. Duplicated select/feedback pattern across two views (suggest extracting a composable)
  3. Missing background-size: cover on login card surface
  4. i18n placeholders in non-EN locales (track as follow-up)

None of these are merge-blocking.

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.

1 participant