fix(tui): keep onboarding status notifications visible - #3378
Open
TheJhyeFactor wants to merge 1 commit into
Open
fix(tui): keep onboarding status notifications visible#3378TheJhyeFactor wants to merge 1 commit into
TheJhyeFactor wants to merge 1 commit into
Conversation
Contributor
Author
|
Hey @meowgorithm, when you have a minute, could you approve the workflows on this one? The build, lint and security runs haven’t started yet. I’d really appreciate a review of the notification ordering once they’re green. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CONTRIBUTING.md.Fixes #3044
What was happening
Hey, I had a look at this on current
main.The status row was drawn before the onboarding dialog. Because that dialog is bottom-aligned, it could overwrite the row containing clipboard confirmations and update notices. The notification still existed in state, but it wasn't visible.
How I reproduced it
I've added a render regression test that puts the UI in onboarding state, draws a dialog over the bottom row, and checks that an update notice remains visible.
Before the change, the test fails because the dialog replaces the entire status row. After the change, it passes consistently.
What changed
During onboarding, the status row is now drawn after the dialog. The normal landing, chat, and initialize render order is unchanged.
The dialog cursor is preserved, and the status layer still hides onboarding help text as before.
Validation
Run on Apple M4,
darwin/arm64, Go 1.26.5:go test -race -failfast ./...go build -race ./...golangci-lint run --path-mode=abs --config=.golangci.yml --timeout=5m./scripts/check_log_capitalization.shgo mod tidy(no diff)go test ./internal/ui/model -run TestDraw_OnboardingStatusRendersAboveDialog -count=10This isn't a performance change, so there isn't a benchmark comparison.
Risk
The change is limited to onboarding render order. When a notification is active, it intentionally owns the final status row instead of letting the onboarding dialog cover it.