Feature/add quit keymaps hint to window#97
Closed
Necrom4 wants to merge 2 commits intojanosmiko:mainfrom
Closed
Conversation
Owner
Author
|
Understandable, I recommend you make a smaller window and center the text then! Also document that "y" can be used to quit, but most people can also guess that. Btw thx for this amazing tool, been recommending it to everyone I work with, never expected such a young/partially-ai-generated tool to already be so solid on its start and basically (finally) replace k9s for me, I never found it intuitive and lfk solves this for me and offers more, love it! Just wish it continues being contributed to regularly so that we know it's still alive, improving and solving bugs. |
Owner
|
Thanks @Necrom4 , I like those ideas better, I'll figure out something. Thanks for the kind words. ❤️ |
6 tasks
janosmiko
added a commit
that referenced
this pull request
Apr 30, 2026
…test Three confirm-overlay renderer comments and one test comment pointed at a "UI conventions" section in CONTRIBUTING.md that was intentionally not added on this branch. Drop the references; keep the rule and the PR #80/#97 history note in place so future contributors still understand why inline `[y] yes [n] no` text shouldn't come back. While here, tighten TestOverlayHintBar_ConfirmDialogsAdvertiseYAndN. The previous loop checked for plain "n" / "y" substrings, which were already satisfied by "Enter" and "cancel" — so removing `Esc/n` from the hint bar would not have failed the test it was written to guard. The check now matches the slash-grouped form (Enter/y, Esc/n) with reverse-order tolerance, locking in the contract the test name claims. Minor: collapse the four-line var/assign block at the top of TestQuitOverlayCentersTextOnMiddleRow into a single := initializer.
janosmiko
added a commit
that referenced
this pull request
Apr 30, 2026
Quit overlay shrinks to a 5-row box (border / pad / "Quit lfk?" / pad / border) with the question on the middle row. lipgloss `Height(N)` counts inner+padding (not border), so the renderOverlayContent arithmetic is qh=3 → outer 5; the renderer's own Align(Center, Center) handles the vertical placement. Closes the floating-text-above-empty-space look that prompted PRs #80 and #97. Inline `[y] yes [n] no` text is removed from the paste confirm overlay, and the paste case is added to overlayHintBarDialog so all confirms keep their keymap in the bottom hint bar — the single source of truth that #80 and #97 were trying to bypass. CONTRIBUTING.md gains a UI Conventions section spelling out the rule so the same PR doesn't keep coming back. Tests pin the visual contract: - TestQuitOverlayCentersTextOnMiddleRow walks the rendered frame and asserts the question sits on row offset 2 of the 5-row dialog. - TestRenderQuitConfirmOverlay/production 26x1 inner subtests pin the exact dimensions used by the production caller. - TestRenderPasteConfirmOverlayHasNoInlineKeyHints fails fast if anyone re-adds inline `[y]/[n]` text to the paste body.
janosmiko
added a commit
that referenced
this pull request
Apr 30, 2026
The confirm overlay handlers (handleConfirmOverlayKey, handleQuitConfirmOverlayKey, handlePasteConfirmKey, the bookmark delete confirms) all accept "y"/"Y" as confirm and "n"/"N" as cancel, but the bottom hint bar only listed Enter and Esc. Users who reach for y/n had to read source — and the gap was exactly what PRs #80 and #97 tried to fix by adding inline `[y] yes [n] no` text inside the dialog. Promote y/n to first-class hints. The confirm/quit/paste dialogs and both bookmark delete confirms now show: Enter/y: <action> | Esc/n: cancel Slash-grouped pairs follow the existing convention used elsewhere (e.g. "q/Esc", "j/k", "ctrl+d/u"). q is the only confirm/cancel alias still left unadvertised — it's a power-user shortcut without a universal expectation, unlike y/n on a yes/no prompt. CONTRIBUTING.md UI Conventions section updated: y/n are now "advertised alongside Enter/Esc" rather than "unadvertised aliases". The closing note about #80/#97 remains so future contributors understand why the hints live in the bar, not in the overlay body. Tests: - TestOverlayHintBar_ConfirmDialogsAdvertiseYAndN locks the contract: every confirm-style overlay (overlayConfirm, overlayQuitConfirm, overlayPasteConfirm, bookmarkModeConfirmDelete, …DeleteAll) must surface Enter, Esc, y, n in the rendered hint bar. Substring checks rather than exact form so a future "y/Enter" rename still passes. - Existing TestOverlayHintBar_ReturnsNonEmpty unchanged: substring assertions on "Enter" still match the new "Enter/y" form.
janosmiko
added a commit
that referenced
this pull request
Apr 30, 2026
…test Three confirm-overlay renderer comments and one test comment pointed at a "UI conventions" section in CONTRIBUTING.md that was intentionally not added on this branch. Drop the references; keep the rule and the PR #80/#97 history note in place so future contributors still understand why inline `[y] yes [n] no` text shouldn't come back. While here, tighten TestOverlayHintBar_ConfirmDialogsAdvertiseYAndN. The previous loop checked for plain "n" / "y" substrings, which were already satisfied by "Enter" and "cancel" — so removing `Esc/n` from the hint bar would not have failed the test it was written to guard. The check now matches the slash-grouped form (Enter/y, Esc/n) with reverse-order tolerance, locking in the contract the test name claims. Minor: collapse the four-line var/assign block at the top of TestQuitOverlayCentersTextOnMiddleRow into a single := initializer.
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
2 participants
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.
Summary
Add
[y] yes [n] nohint to "Quit lfk?" window.The window wasn't intuitive to me at first, there was a blank space on the lower half, it almost felt like there was a hint supposed to be there, so I added it. Without it one would the typicals like
q/ybut may not see (like I did) that there was a dark hint on the bottom bar saying to pressEnter.Type of change
Related issue
Changes
Testing
go build ./...succeedsgo test ./...passesgolangci-lint runpassesChecklist
feat:,fix:,refactor:,docs:,test:,chore:,perf:,ci:)docs/updated when user-visible behavior or config changeddocs/keybindings.mdand the in-app help screen updated when keybindings changedmain)Screenshots / recordings