Skip to content

fix(structure): inherit row emphasis propagation and deleted-row tint from DataGridRowView#1181

Closed
datlechin wants to merge 3 commits intomainfrom
fix/structure-row-emphasis-propagation
Closed

fix(structure): inherit row emphasis propagation and deleted-row tint from DataGridRowView#1181
datlechin wants to merge 3 commits intomainfrom
fix/structure-row-emphasis-propagation

Conversation

@datlechin
Copy link
Copy Markdown
Member

Summary

Structure tab's StructureRowViewWithMenu now subclasses DataGridRowView instead of NSTableRowView so it inherits the emphasis-propagation logic that PR #1135 added for data-tab rows.

Before

Selecting a row in the Structure tab tinted the focused cell's text correctly (the cell that AppKit invalidated to draw the focus ring), but the other cells in the same row stayed at their unselected text color. The user reported "the active row it correct text color on focus cell, other cell is wrong color, also other issues."

Root cause

DataGridCellView (the direct-draw NSView cell from PR #1135) reads isSelected && isEmphasized from its enclosing row view in viewWillDraw to pick alternateSelectedControlTextColor vs the regular text color. But it only redraws when needsDisplay is set on it. AppKit doesn't invalidate sibling cells when the row's selected/emphasized state changes — that propagation is the row view's job.

DataGridRowView already does this:

override var isSelected: Bool { didSet { invalidateCellSubviews() } }
override var isEmphasized: Bool { didSet { invalidateCellSubviews() } }
private func invalidateCellSubviews() {
    for subview in subviews where subview is DataGridCellView { subview.needsDisplay = true }
}

StructureRowViewWithMenu: NSTableRowView had no such overrides, so structure-tab cells stuck on their unselected color until something else invalidated them.

Fix

  • DataGridRowView drops final to allow subclassing.
  • StructureRowViewWithMenu now extends DataGridRowView. It removes the duplicated coordinator and rowIndex properties (inherited from the parent) and routes its existing isRowDeleted flag through applyVisualState(_:) so deleted rows pick up the same red tint the data tab uses.

Side benefits: structure rows also gain the layer-backing setup (wantsLayer, canDrawSubviewsIntoLayer, disabled implicit layer animations) that PR #1135 added for scroll perf, and the deleted-row tint that previously was tracked but never drawn in the structure tab.

Test plan

  • Open Structure tab on a table with multiple columns. Click a row. Confirm every cell in the row uses the emphasized text color, not just the focused one.
  • Click into the data tab and back; structure-tab selection text color stays correct on subsequent re-selection.
  • Mark a column for deletion in the Structure tab. Confirm the row gets the red tint (matching the data-tab deleted-row appearance).
  • Scroll a long structure list (e.g. a wide table); no perf regression.
  • Same checks in CreateTableView (which uses the same row view).
  • swiftlint --strict clean.

@datlechin
Copy link
Copy Markdown
Member Author

Rolling into a larger Structure-tab fix bundle that also adds applyVisualState wiring, reloadVersion observation, Add Row routing, Cmd+Z/Edit menu undo, and dead-code cleanup. New PR coming.

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.

1 participant