Skip to content

docs: Add section about up-modal#1330

Merged
CJ42 merged 16 commits intomainfrom
feat/up-modal-docs-DEV-15848
Mar 12, 2026
Merged

docs: Add section about up-modal#1330
CJ42 merged 16 commits intomainfrom
feat/up-modal-docs-DEV-15848

Conversation

@dzbo
Copy link
Contributor

@dzbo dzbo commented Mar 3, 2026

@frozeman
Copy link
Member

frozeman commented Mar 3, 2026

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2026

Deployed with Cloudflare Pages ☁️ 🚀 🆗

Copy link
Contributor

@leo-assistant-chef leo-assistant-chef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📋 Review — docs (PR #1330)

Overall this is a solid first pass — well-structured, good framework coverage, and the API reference table is clean. A few issues worth addressing before merge, ranging from a subtle bug in the React example to some missing dependency notes that will trip up developers.

Summary of findings:

  • 🔴 1 bug: React example has a race condition (module-level connector var)
  • 🟠 2 footguns: enableTestnet: true default + missing peer dep info
  • 🟡 3 clarity gaps: @lukso/core install undocumented, undefined var in i18n example, Svelte version ambiguity
  • 🔵 1 structural: missing index/overview page for the section

Inline comments below on specific lines.

@leo-assistant-chef
Copy link
Contributor

👨🏻‍🍳 Kitchen Review — @lukso/up-modal docs

Yo Dominik! Leo here — assistant chef at BuilderLabs, helping Jean keep the LUKSO kitchen running smooth 🦁🔥

Overall this is a solid plate — well-structured, good framework coverage, the API table is clean. A few things to fix before we serve this to developers though. Nothing that can't be plated with a quick pass.

Order of fixes before service:

  • 🔴 React example has a race condition (silent failure — the worst kind)
  • 🟠 enableTestnet: true default will catch prod devs off-guard
  • 🟠 @wagmi/core peer dep not mentioned in install docs
  • 🟡 @lukso/core missing from install step — i18n page will error immediately
  • 🟡 germanMessages is undefined in the runtime locale switching example
  • 🟡 Vue/Nuxt tab label — example is Nuxt-only, pure Vue 3 devs will be lost
  • 🔵 Svelte on:click is v4 syntax — Svelte 5 uses onclick

Inline comments on each specific spot above. Let's get this to the pass! 🥔

Copy link
Member

@CJ42 CJ42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, added some extra review comments

@dzbo dzbo requested a review from leo-assistant-chef March 4, 2026 11:23
@dzbo dzbo requested a review from CJ42 March 6, 2026 12:46
@dzbo dzbo changed the title feat: Add section about up-modal docs: Add section about up-modal Mar 6, 2026
@CJ42 CJ42 merged commit 46fe278 into main Mar 12, 2026
3 checks passed
@CJ42 CJ42 deleted the feat/up-modal-docs-DEV-15848 branch March 12, 2026 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants