Skip to content

Fixes #5365. Fix Markdown auto-focus, Home/End keys, and Deepdives Tab nav - #5370

Merged
tig merged 6 commits into
developfrom
tig/fix-markdown-auto-focus-last-link
May 23, 2026
Merged

Fixes #5365. Fix Markdown auto-focus, Home/End keys, and Deepdives Tab nav#5370
tig merged 6 commits into
developfrom
tig/fix-markdown-auto-focus-last-link

Conversation

@tig

@tig tig commented May 22, 2026

Copy link
Copy Markdown
Member

Summary

Four fixes for the Markdown view and Deepdives scenario:

1. Fix Markdown auto-focuses last link on initial render (#5365)

Root cause: MarkdownTable.BuildLinkRegions() set CanFocus = true before the table was added via Add(). View.AddAt() auto-focuses any focusable subview when the SuperView has focus — so each successive table stole focus, leaving the last one focused and scrolling the viewport to the bottom.

Fix: Removed CanFocus assignment from BuildLinkRegions(). Instead, Markdown.OnSubViewLayout() manages table focusability after all Add() calls complete, then clears any auto-focused table to keep the viewport at the top.

2. Add Home/End key support to Markdown

Home/End keys now scroll to the top/bottom of the document (same as Ctrl+Home/End which were already bound). Markdown is a read-only view with no cursor, so plain Home/End should behave identically to Ctrl+Home/End.

3. Fix Deepdives Tab navigation

FrameView defaults to TabBehavior.TabGroup, which means Tab cycles within the frame and F6/Ctrl+Tab is required to move between frames. Set both FrameViews to TabBehavior.TabStop so Tab naturally navigates between the doc list and markdown viewer.

4. Prevent CursorDown/Up from escaping Docs list

When the ListView reaches the bottom/top of the list and CursorDown/Up is unhandled, it propagates to the Application level where it's aliased to NextTabStop, shifting focus to the markdown viewer. Added KeyDownNotHandled handler on the list to consume arrow keys at boundaries.

Testing

  • Added regression test: Initial_Render_With_Table_Links_Does_Not_Scroll_Viewport
  • All 93 MarkdownView + 38 MarkdownTable tests pass

…al render

MarkdownTable.BuildLinkRegions() was setting CanFocus = true during layout,
which caused View.AddAt() to auto-focus the table when the parent Markdown
view had focus. This scrolled the viewport to the last table with links.

Fix: Remove CanFocus assignment from BuildLinkRegions (keep TabStop only).
The parent Markdown view now sets CanFocus on tables after Add() completes,
then clears any auto-focused table to prevent unwanted viewport scrolling.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c2618a4219

ℹ️ 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".

Comment thread Terminal.Gui/Views/Markdown/MarkdownTable.cs Outdated
- Markdown: Add Command.LeftStart/RightEnd handlers so Home/End keys
  scroll to top/bottom (Ctrl+Home/End already worked via Command.Start/End)
- Deepdives: Set TabStop = TabBehavior.TabStop on both FrameViews so Tab
  navigates between the list and markdown viewer (FrameView defaults to
  TabGroup which requires F6/Ctrl+Tab)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@tig tig changed the title Fixes #5365. Prevent Markdown table links from auto-focusing on initial render Fixes #5365. Fix Markdown auto-focus, Home/End keys, and Deepdives Tab nav May 22, 2026
tig and others added 3 commits May 22, 2026 10:58
When the ListView is at the bottom/top and CursorDown/Up is not handled,
it propagates to the Application level where it maps to NextTabStop,
shifting focus to the markdown viewer. Add KeyDownNotHandled handler to
consume arrow keys at boundaries.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When MarkdownTable is used outside of a Markdown parent, BuildLinkRegions
must set CanFocus = hasLinks so that tables with links are keyboard-
navigable. The Markdown parent's OnSubViewLayout already handles clearing
unwanted auto-focus that results from this.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…focus

Root cause: OnSubViewLayout set table.CanFocus=true AFTER tables were
already in the hierarchy. The CanFocus setter's auto-focus guard
(SuperView is { Focused: null }) fired because the Markdown view had no
focused child, calling SetFocus and stealing focus from the ListView.

Additionally, when the Markdown view gains focus, SetHasFocusTrue calls
AdvanceFocus(Forward, null) which auto-focuses the first focusable table.

Fix:
- Set CanFocus in BuildLinkRegions (before Add, when SuperView is null -
  the setter's guard doesn't fire)
- Remove redundant CanFocus loop from OnSubViewLayout
- Cancel auto-advance (behavior==null) in OnAdvancingFocus so gaining
  focus doesn't drill into table SubViews; only Tab navigation does
- Remove incorrect KeyDownNotHandled workaround from Deepdives

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR targets Markdown view navigation/focus behavior (including a regression for #5365), adds Home/End scrolling support for Markdown, and adjusts the UICatalog “Deepdives” scenario’s tab navigation.

Changes:

  • Add Markdown commands so Home/End (without Ctrl) scroll to top/bottom like Ctrl+Home/End.
  • Attempt to prevent initial auto-focus/auto-scroll behavior involving Markdown tables/links, and add a regression test for #5365.
  • Change Deepdives scenario FrameViews to TabBehavior.TabStop to make Tab move between panes.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
Tests/UnitTestsParallelizable/Views/Markdown/MarkdownViewTests.cs Adds regression test for initial render viewport position/focus with table links.
Terminal.Gui/Views/Markdown/MarkdownView.Mouse.cs Adds Home/End command handlers and changes focus-advance behavior when focus is gained.
Terminal.Gui/Views/Markdown/MarkdownTable.cs Adds HasLinks and updates link-region focusability logic/comments.
Terminal.Gui/Views/Markdown/Markdown.cs Adds logic in layout to clear table auto-focus after rebuilding subviews.
Examples/UICatalog/Scenarios/Deepdives.cs Sets both FrameViews to TabBehavior.TabStop to change Tab traversal behavior.
Comments suppressed due to low confidence (1)

Examples/UICatalog/Scenarios/Deepdives.cs:54

  • PR description mentions consuming CursorUp/Down at ListView boundaries (KeyDownNotHandled) to prevent focus from jumping to the markdown viewer, but this scenario code only changes TabStop on the frames. If that arrow-key boundary handling is still required, it looks missing here and should be added to _docList to match the described behavior.
        FrameView listFrame = new ()
        {
            Title = "_Docs",
            X = 0,
            Y = 0,
            Width = 30,
            Height = Dim.Fill (1),
            TabStop = TabBehavior.TabStop
        };

        _docList = new ListView { Width = Dim.Fill (), Height = Dim.Fill () };

        _docList.ValueChanged += OnDocListValueChanged;
        listFrame.Add (_docList);

Comment thread Terminal.Gui/Views/Markdown/MarkdownTable.cs
Comment thread Terminal.Gui/Views/Markdown/Markdown.cs
Comment thread Tests/UnitTestsParallelizable/Views/Markdown/MarkdownViewTests.cs
…roach

- BuildRenderedLines: set CanFocus=false before Add() to prevent AddAt
  auto-focus, then re-enable with HasFocusChanging cancel handler after
  all tables are added (prevents CanFocus setter's auto-focus guard)
- OnSubViewLayout: keep cleanup as safety net for edge cases where a
  table still gets focused; reset _activeLinkIndex to avoid link nav
  state leaking from layout
- MarkdownTable.BuildLinkRegions: updated comment to accurately describe
  the CanFocus behavior for standalone vs hosted usage
- Added 3 regression tests covering: multiple layout cycles, content
  change while focused, and multi-table cascade prevention
- Strengthened original regression test with multiple layout/draw cycles

All 17,076 parallel tests pass (96 MarkdownView + 38 MarkdownTable).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@tig
tig merged commit dbb7e4e into develop May 23, 2026
13 checks passed
@tig
tig deleted the tig/fix-markdown-auto-focus-last-link branch May 23, 2026 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants