feat: add optional boot splash settings#224
Conversation
- add smooth retro boot splash shown once per new app version - add config-backed Settings screen for boot splash preference - keep context picker focused on context workflows Closes #222
|
Warning Review limit reached
More reviews will be available in 16 minutes and 42 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
WalkthroughThis PR implements a quirky 80s-style bootup splash animation that displays briefly during TUI startup, with optional configuration to disable or always show it. A new Settings screen lets users toggle the preference per launch. The startup sequence, context loading, and global navigation are updated to accommodate the bootup screen flow. ChangesTUI Boot Splash & Settings
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR successfully adds a configurable retro bootup splash screen feature with proper settings integration. The implementation is clean and well-structured:
- Bootup animation renders correctly with frame-based progression
- Settings screen allows toggling the splash on/off
- Config persistence works properly with YAML marshaling
- State management flows correctly through the TUI lifecycle
- Error handling is appropriate throughout
No blocking defects found. The code is ready to merge.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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 `@internal/app/app_test.go`:
- Around line 236-263: Add a new test (e.g.,
TestSettingsTogglesBootSplashPreference_Error) that mirrors
TestSettingsTogglesBootSplashPreference but makes configSetBootSplashEnabledFn
return a non-nil error; instantiate the model with New(cfg, "config.yaml",
"dev"), set m.screen = screenSettings, call m.Update(tea.KeyMsg{Type:
tea.KeyEnter}) and assert the returned Model has m.screen == screenError (or the
exported equivalent) and that the error branch is taken (check any error
message/state on Model), and ensure you restore the original
configSetBootSplashEnabledFn in a defer just like the existing test to avoid
cross-test pollution.
In `@internal/app/app.go`:
- Around line 486-493: The global uppercase "S" shortcut handler is firing
before screen-specific input and should skip screens that accept text entry
(e.g., screenContextAdd) so it doesn't steal input; update the conditional that
checks msg.String() == "S" and m.filterTI.Focused() to also exclude text-entry
screens by adding checks like m.screen != screenContextAdd (and any other
text-entry screens) before calling m.deactivateFilter(), setting
m.settingsPrevScreen and switching to screenSettings, ensuring screen-specific
input handlers still receive the key.
In `@internal/app/screen_bootup.go`:
- Line 17: Replace the hardcoded banner string passed to dimStyle.Render with a
formatted string that uses the model's currentVersion (m.currentVersion) and
falls back to a sensible default (e.g., "v0.0.0" or "unknown") when
m.currentVersion is empty; locate the call to dimStyle.Render in
screen_bootup.go and change it to construct the banner using m.currentVersion so
the splash header reflects the actual app version at runtime.
In `@internal/app/screen_context.go`:
- Around line 40-42: The handler is overwriting active screens during background
startup because it unconditionally sets m.screen = screenContextPicker when
m.screen != screenBootup; instead, change the message from loadContexts() to
carry an explicit startup flag (e.g., contextsLoadedMsg{startup bool}) and only
switch screens when the loadContexts result is from startup and the model is
still on boot (e.g., if msg.startup && m.screen == screenBootup then m.screen =
screenContextPicker); do not change m.screen for non-startup loads so
user-opened screens like Settings aren't bounced.
In `@internal/app/screen_settings.go`:
- Around line 66-77: The current fmt.Sprintf call pads ANSI escape sequences
instead of visible widths; replace the fmt.Sprintf usage in the loop over items
with lipgloss width-aware rendering: build a name cell using
nameStyle.Copy().Width(18).Render(item.name) (or
lipgloss.NewStyle().Width(18).Render(nameStyle.Render(item.name))) and a value
cell via valueStyle.Render(item.value), then write the row with
panel.WriteString(prefix + nameCell + " " + valueCell + "\n"); keep the
description line using dimStyle.Render(item.description) as before and preserve
the selection logic that sets prefix/nameStyle/valueStyle when i ==
m.settingsIdx.
In `@internal/app/service_list.go`:
- Around line 89-99: The toggleBootSplash function currently flips m.bootSplash
and m.cfg.BootSplash before persisting; change it to compute the desired new
value first (e.g., newVal := !m.bootSplash), call
configSetBootSplashEnabledFn(m.configPath, newVal) and only on success assign
m.bootSplash = newVal and if m.cfg != nil set m.cfg.BootSplash = newVal; keep
the existing configPath empty-string check and return any error from
configSetBootSplashEnabledFn without mutating in-memory state.
In `@internal/config/config_test.go`:
- Around line 179-217: Add a regression test that verifies disabling boot splash
survives save/load: call writeUnicConfig to create a base config (preserving an
existing field like default_region), invoke SetBootSplashEnabled(path, false),
then Load(nil, nil, path) and assert cfg.BootSplash is false and other fields
(e.g. cfg.Region) are preserved; place this alongside
TestSetBootSplashEnabledWritesConfig and
TestSetBootSplashSeenVersionWritesConfig to catch the YAML-contract
explicit-disable bug affecting SetBootSplashEnabled and the BootSplash field
round-trip.
In `@internal/config/config.go`:
- Around line 41-44: The fileUI struct's BootSplash boolean uses `omitempty`,
collapsing "unset" and "false" and preventing Load and SetBootSplashEnabled from
distinguishing an explicit disable; change BootSplash to a tri-state (e.g.,
*bool) so nil = unset, true/false = explicit, and remove omitempty only when you
truly want to omit unset; update any related structs/fields mentioned around
lines 74-87 and 638-655 to the same tri-state pattern, and adjust
SetBootSplashEnabled, Load (and any marshal/unmarshal logic) to handle nil as
"use default behavior" while true/false represent explicit choices. Ensure
LastBootSplashVersion remains a string but keep BootSplash as *bool to preserve
explicit disabled state.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 8376f7f8-8274-4c99-ab4e-acf35de8af1b
📒 Files selected for processing (11)
README.mdinternal/app/app.gointernal/app/app_test.gointernal/app/help.gointernal/app/messages.gointernal/app/screen_bootup.gointernal/app/screen_context.gointernal/app/screen_settings.gointernal/app/service_list.gointernal/config/config.gointernal/config/config_test.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Use lipgloss for styled TUI output — column-aligned tables with dimmed labels in Go implementation files
Implement scroll windowing with formula:visibleLines := max(m.height-N, 5)in Go TUI implementation
Files:
internal/app/messages.gointernal/app/service_list.gointernal/config/config_test.gointernal/app/screen_settings.gointernal/app/screen_bootup.gointernal/config/config.gointernal/app/help.gointernal/app/app.gointernal/app/screen_context.gointernal/app/app_test.go
⚙️ CodeRabbit configuration file
**/*.go: For Go reviews, look beyond compilation and prioritize nil pointer risks,
context propagation, AWS SDK pagination, error wrapping, deterministic
sorting, and stable table/detail rendering. For new AWS service work,
verify that repository interfaces, model mapping, app integration, and
tests are updated together.
Files:
internal/app/messages.gointernal/app/service_list.gointernal/config/config_test.gointernal/app/screen_settings.gointernal/app/screen_bootup.gointernal/config/config.gointernal/app/help.gointernal/app/app.gointernal/app/screen_context.gointernal/app/app_test.go
internal/app/**
⚙️ CodeRabbit configuration file
internal/app/**: For Bubble Tea screen changes, verify message routing, key handling,
filter target resets, height-based windowing, help text, and back/home
navigation against the existing screen patterns.
Files:
internal/app/messages.gointernal/app/service_list.gointernal/app/screen_settings.gointernal/app/screen_bootup.gointernal/app/help.gointernal/app/app.gointernal/app/screen_context.gointernal/app/app_test.go
README.md
📄 CodeRabbit inference engine (CLAUDE.md)
README.md: When adding, modifying, or deleting features, always updateREADME.mdin parallel with code changes
UpdateCurrently Implemented Featurestable in README.md: add new services/features, update status changes (🚧→✅), remove deleted items
UpdateTUI Key Bindingstable in README.md when key bindings are added, changed, or deleted
UpdateUsagesection in README.md when new CLI commands or flags are added
UpdateConfigurationsection in README.md when configuration format changes
Files:
README.md
⚙️ CodeRabbit configuration file
README.md: Verify that README changes match actual CLI/TUI behavior and that
Currently Implemented Features, TUI Key Bindings, Usage, and
Configuration content stay aligned with code changes.
Files:
README.md
**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
Tests use mock client interfaces (see
rds_test.gopattern) in Go test files
Files:
internal/config/config_test.gointernal/app/app_test.go
⚙️ CodeRabbit configuration file
**/*_test.go: Check that tests cover API errors, mapping edge cases, and navigation
state transitions, not only happy paths. Prefer mock-based tests that do
not depend on external AWS calls.
Files:
internal/config/config_test.gointernal/app/app_test.go
2 participants
Summary
Related Issues
Closes #222
Validation
Checklist
Summary by CodeRabbit
Release Notes
New Features
Documentation