Visual polish: animated wordmark, install script, interactive skill wizard#208
Visual polish: animated wordmark, install script, interactive skill wizard#208
Conversation
There was a problem hiding this comment.
Pull request overview
Adds brand-focused terminal polish and interactivity across the CLI: animated braille wordmark, a richer installer banner, and an interactive basecamp skill wizard (with non-interactive fallback), plus Escape-to-cancel support in core TUI prompts.
Changes:
- Introduces new Basecamp braille wordmark rendering + animation utilities (with tests).
- Updates the install script to show a banner animation and standardized colored step/success/error output.
- Adds an interactive skill installation wizard and shows the animated wordmark in quickstart/setup flows.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/install.sh | Adds colored status helpers and an animated installer banner. |
| internal/tui/forms.go | Adds Escape-to-cancel keymap for Confirm/Input/Select forms. |
| internal/tui/brand.go | Adds the braille wordmark constants and a renderer. |
| internal/tui/brand_anim.go | Adds animated bottom-up color sweep wordmark rendering. |
| internal/tui/brand_test.go | Tests wordmark dimensions and RenderWordmark behavior. |
| internal/commands/quickstart.go | Shows animated wordmark on interactive basecamp invocation. |
| internal/commands/wizard.go | Shows animated wordmark on setup wizard welcome screen. |
| internal/commands/skill.go | Adds interactive basecamp skill picker + path normalization/expansion helpers. |
| internal/commands/skill_test.go | Adds tests for skill path normalization/expansion helpers. |
| README.md | Adds braille art in a code fence near the top of the README. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f4f4377cd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
4 issues found across 10 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="internal/commands/wizard.go">
<violation number="1" location="internal/commands/wizard.go:288">
P2: Hardcoded `os.Stdout` is inconsistent with the `cmd.OutOrStdout()` pattern used for the same `AnimateWordmark` call in `quickstart.go`. Consider passing an `io.Writer` parameter to `showWelcome` (the caller in `runWizard` has access to `cmd`) so output can be redirected in tests and stays consistent with the rest of the codebase.</violation>
</file>
<file name="internal/commands/skill.go">
<violation number="1" location="internal/commands/skill.go:158">
P2: `normalizeSkillPath` can produce a double-nested `basecamp/basecamp/SKILL.md` path. The first condition is case-sensitive (`SKILL.md`), but the second `.md` branch is case-insensitive and always inserts a `basecamp/` segment. A user entering e.g. `~/skills/basecamp/skill.md` would get `~/skills/basecamp/basecamp/SKILL.md`. Consider making the `SKILL.md` check case-insensitive, or checking whether the parent dir is already `basecamp` before inserting it.</violation>
</file>
<file name="scripts/install.sh">
<violation number="1" location="scripts/install.sh:224">
P2: Cursor-movement escape sequences in `show_banner` are emitted even when stdout is not a TTY. The width check `cols >= 44` always passes for non-TTY output because `tput cols` fails and falls back to 80. Add a `[[ -t 1 ]]` guard to the outer condition so the animated braille art (with its cursor repositioning and sleeps) is only shown on interactive terminals.</violation>
</file>
<file name="internal/tui/forms.go">
<violation number="1" location="internal/tui/forms.go:11">
P2: Inconsistent escape-to-cancel behavior: `Confirm`, `Input`, and `Select` now support Escape to cancel, but sibling functions in the same file (`ConfirmDangerous`, `InputRequired`, `SelectWithDescription`, `MultiSelect`, `ConfirmSetDefault`) still run the field directly without the `escKeyMap()`. Users will experience inconsistent behavior where Escape works in some prompts but not others.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Render the Basecamp mountain logo as braille art (14×32 chars) with a fixed brand yellow (#e8a217) independent of theme. AnimateWordmark paints the logo via a rising gradient sweep — ghost gray through warm amber stages — then reveals "Basecamp" letter by letter to the right. Dark/light intermediate palettes ensure the ghost silhouette is visible on both backgrounds.
Display the brand animation on interactive TTY for both `basecamp` (no args) and `basecamp setup`. Non-interactive/piped output is unaffected.
Line-by-line logo reveal in yellow, then "Basecamp" typed out to the right via cursor repositioning. Colored step/success/error prefixes with NO_COLOR and terminal width guards.
…path Make `basecamp skill` an interactive picker on TTY with destinations for Claude Code, OpenCode, Codex, and the shared ~/.agents/skills location. Falls back to printing skill content when piped. Add escape key as cancel alongside ctrl+c in all TUI form widgets (Select, Confirm, Input) via a custom huh keymap.
Replace hardcoded BFS paint order with a strategy registry supporting warnsdorff (finger-painting DFS), outline-in (reverse warnsdorff), radial (concentric rings from centroid), and scanline (typewriter). Default is outline-in. Set BASECAMP_ANIM=warnsdorff (etc.) to switch. Refactors shared helpers: wordmarkGrid, trailStyles, makeRevealGrid, findInteriorPeak. Batches cells into ~30 frames regardless of strategy.
AnimateWordmarkAsync starts the animation in a background goroutine and returns a mutex-protected writer. Output written through it appears below the animating logo. Quickstart routes app.OK through the AnimWriter so summary and breadcrumbs render during animation. Wizard writes welcome text through it then waits before interactive prompts.
Config-driven format=json could bypass the machine-output check, printing the human checklist even in machine mode. Add IsMachineOutput guard alongside IsInteractive.
Skip the animated cursor-repositioning path when NO_COLOR is set, even on a TTY. Cursor movement escapes violate the NO_COLOR contract.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Three fixes from review: 1. AnimWriter now counts visual terminal rows (not just \n bytes) using lipgloss.Width for ANSI-aware measurement and terminal width for soft-wrap detection. Prevents repaint-into-text in narrow panes. 2. Unknown BASECAMP_ANIM values fall back to DefaultAnimation instead of silently disabling animation entirely. 3. Add 13 tests covering visualLines (wrapping, ANSI escapes, partial lines, continuations, exact-fit, zero-cols fallback) and AnimWriter row accounting.
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="internal/tui/brand_anim.go">
<violation number="1" location="internal/tui/brand_anim.go:83">
P3: The doc comment says unknown strategy names fall back to static render, but the code now falls back to `DefaultAnimation` (animated). Update the comment to match the new behavior, or a future reader may misunderstand the fallback path.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
1. Quickstart preserves the effective output format (e.g. Markdown via --md) instead of forcing FormatStyled when routing through AnimWriter. 2. visualLines partial-line branch now uses > (not >=) for the wrap threshold, matching terminal deferred-wrap behavior: the cursor stays at the right margin until the next printable character. pendingW=cols encodes the deferred state. 3. Add tests: exact-fit partial line (no \n), exact-fit then newline, double exact-fit, invalid BASECAMP_ANIM fallback to default strategy, valid strategy passthrough, DefaultAnimation in AnimationNames.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
When AnimateWordmarkAsync falls back to static (non-TTY destination), it returns the original writer. Detect this with an identity check and leave app.Output untouched so its format resolution stays tied to the real destination writer. Prevents ANSI-styled output leaking to a non-TTY cmd.OutOrStdout() when os.Stdout happens to be a TTY. Replace two false-positive strategy tests (NoColorTheme hit static render before the lookup) with direct animations-map tests that actually exercise the fallback logic.
Update AnimateWordmarkWith comment to reflect that unknown strategy names fall back to DefaultAnimation (not static render). Fix stray blank line and mis-indented fmt.Fprintln in showSuccess agent checks.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
#e8a217), independent of themebasecampinvocation andbasecamp setupwizard (TTY only)basecamp skillshows a multi-agent picker (Agents shared, Claude Code, OpenCode, Codex, custom path) on TTY, falls back to printing skill content when pipedTest plan
make build && ./bin/basecamp— animated logo + quick-start on TTY./bin/basecamp setup— animated logo in setup wizard./bin/basecamp skill— interactive picker, escape cancels./bin/basecamp skill | head— prints skill content (non-interactive)bash scripts/install.sh— banner animation with "Basecamp" to the rightmake test— all unit tests passmake test-e2e— all e2e tests passSummary by cubic
Polished the CLI’s brand and onboarding with a pluggable animated braille wordmark, a friendlier install banner, and a TTY‑only interactive skill wizard with Esc‑to‑cancel. Animations run only on TTY; piped/JSON/agent output is unchanged, output format is preserved, and we only reroute output when the animation actually runs to avoid ANSI leaks.
New Features
basecampand duringsetup; AnimWriter keeps output below the logo, and the wizard waits before prompts; setup shows the rich checklist only in interactive mode—machine output returns the JSON envelope; preserves the effective output format (e.g. --md).~/.agents/skills/basecamp/SKILL.md; non‑interactive prints SKILL.md.Bug Fixes
Written for commit 7e3cc7f. Summary will update on new commits.