Skip to content

🎨 Palette: [UX improvement] Expand shortcut folder tooltip hit area - #438

Open
google-labs-jules[bot] wants to merge 2 commits into
mainfrom
palette/improve-settings-tooltip-area-4167422764312012505
Open

🎨 Palette: [UX improvement] Expand shortcut folder tooltip hit area#438
google-labs-jules[bot] wants to merge 2 commits into
mainfrom
palette/improve-settings-tooltip-area-4167422764312012505

Conversation

@google-labs-jules

Copy link
Copy Markdown
Contributor

💡 What: Moved the ToolTipService.ToolTip from the inner TextBlock to the parent StackPanel in SettingsWindow.xaml and added Background='Transparent'.

🎯 Why: Increases the hover target area for the folder path tooltip, so the tooltip displays when the user hovers over any part of the folder name or path list item.

♿ Accessibility: Improves usability for users with motor difficulties by expanding the interactable hit area for the tooltip.


PR created automatically by Jules for task 4167422764312012505 started by @mikekthx

@google-labs-jules

Copy link
Copy Markdown
Contributor Author

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@google-labs-jules
google-labs-jules Bot requested a review from mikekthx as a code owner July 17, 2026 11:36
@github-actions github-actions Bot added the ui Front-end changes, WinUI layouts, styling (e.g., Acrylic), or system tray updates. label Jul 17, 2026
@mikekthx

Copy link
Copy Markdown
Owner

Code Review — PR #438

What this PR does: Moves ToolTipService.ToolTip="{x:Bind Path}" from the inner path TextBlock to the parent StackPanel, and adds Background="Transparent" to the StackPanel. The intent is to expand the hover target so the folder-path tooltip triggers over the full two-line item area, not just the narrow path text row.

Overall assessment: The change is minimal, compiles correctly, and achieves its stated goal. {x:Bind Path} without a converter is valid inside a Window-rooted DataTemplate (per project sharp-edge notes). Adding Background="Transparent" to the StackPanel correctly enlarges the pointer hit-test area so the tooltip triggers on the panel's full layout bounds. No drag-reorder interference (drag is handled at the ListViewItem container level, above this panel; the parent Grid already has Background="Transparent"). WinUI 3's ToolTipService walks the visual tree upward, so the path tooltip still shows when hovering directly over the path TextBlock even though that element no longer declares the attached property directly.

One issue to address:

Because ToolTipService walks up to the first ancestor with a tooltip, hovering the bold label TextBlock (which displays the folder's display name, is never truncated, and is semantically unrelated to the path) also triggers the path tooltip. A user hovering "My Games" will see a tooltip reading C:\Users\user\Desktop\Shortcuts\Games, which is contextually misleading — the hovered text is fully visible and the tooltip describes something else entirely.

The original scoping — tooltip only on the TextBlock that has TextTrimming="CharacterEllipsis" — was intentional: it follows the standard XAML idiom of revealing elided text on hover of the clipped element. If the goal is a larger trigger area, a cleaner fix would be to widen the path TextBlock's hit area (e.g. MinHeight, padding, or IsHitTestVisible="True" with a Background="Transparent") rather than moving the tooltip to the shared parent.

@jules please address the issues noted above

Comment thread SettingsWindow.xaml Outdated
@google-labs-jules

Copy link
Copy Markdown
Contributor Author

Code Review — PR #438

What this PR does: Moves ToolTipService.ToolTip="{x:Bind Path}" from the inner path TextBlock to the parent StackPanel, and adds Background="Transparent" to the StackPanel. The intent is to expand the hover target so the folder-path tooltip triggers over the full two-line item area, not just the narrow path text row.

Overall assessment: The change is minimal, compiles correctly, and achieves its stated goal. {x:Bind Path} without a converter is valid inside a Window-rooted DataTemplate (per project sharp-edge notes). Adding Background="Transparent" to the StackPanel correctly enlarges the pointer hit-test area so the tooltip triggers on the panel's full layout bounds. No drag-reorder interference (drag is handled at the ListViewItem container level, above this panel; the parent Grid already has Background="Transparent"). WinUI 3's ToolTipService walks the visual tree upward, so the path tooltip still shows when hovering directly over the path TextBlock even though that element no longer declares the attached property directly.

One issue to address:

Because ToolTipService walks up to the first ancestor with a tooltip, hovering the bold label TextBlock (which displays the folder's display name, is never truncated, and is semantically unrelated to the path) also triggers the path tooltip. A user hovering "My Games" will see a tooltip reading C:\Users\user\Desktop\Shortcuts\Games, which is contextually misleading — the hovered text is fully visible and the tooltip describes something else entirely.

The original scoping — tooltip only on the TextBlock that has TextTrimming="CharacterEllipsis" — was intentional: it follows the standard XAML idiom of revealing elided text on hover of the clipped element. If the goal is a larger trigger area, a cleaner fix would be to widen the path TextBlock's hit area (e.g. MinHeight, padding, or IsHitTestVisible="True" with a Background="Transparent") rather than moving the tooltip to the shared parent.

@jules please address the issues noted above

Thank you for the review. I have updated the XAML structure as requested: ToolTipService.ToolTip is now removed from the parent StackPanel. Instead, the inner path TextBlock is now wrapped in a Grid with Background="Transparent" and ToolTipService.ToolTip="{x:Bind Path}". This correctly expands the tooltip hit area specifically for the path, without causing the path tooltip to inappropriately appear when hovering over the unrelated folder display name label.

Comment thread SettingsWindow.xaml
Comment on lines +53 to +56
<Grid Background="Transparent" ToolTipService.ToolTip="{x:Bind Path}" Padding="0,2,0,2">
<TextBlock Text="{x:Bind Path}" FontSize="12" Foreground="{ThemeResource TextFillColorSecondaryBrush}"
TextTrimming="CharacterEllipsis" />
</Grid>

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Grid is a multi-child layout panel; Border is the idiomatic single-child decorator for adding padding/background. Functionally identical here, but Border is lighter and makes the intent clearer.

Suggested change
<Grid Background="Transparent" ToolTipService.ToolTip="{x:Bind Path}" Padding="0,2,0,2">
<TextBlock Text="{x:Bind Path}" FontSize="12" Foreground="{ThemeResource TextFillColorSecondaryBrush}"
TextTrimming="CharacterEllipsis" />
</Grid>
<Border Background="Transparent" ToolTipService.ToolTip="{x:Bind Path}" Padding="0,2,0,2">
<TextBlock Text="{x:Bind Path}" FontSize="12" Foreground="{ThemeResource TextFillColorSecondaryBrush}"
TextTrimming="CharacterEllipsis" />
</Border>

@mikekthx

Copy link
Copy Markdown
Owner

Code Review

What this PR does: Expands the tooltip hit area for shortcut folder path rows in the Settings window. The path TextBlock (which trims with ellipsis) is now wrapped in a transparent Grid carrying the ToolTipService.ToolTip attachment, plus Padding="0,2,0,2" to extend the pointer-trigger zone above and below the text.

Thread resolution: The previous unresolved thread raised the concern that the tooltip was on the parent StackPanel, so hovering the bold label row would show the raw path string — contextually misleading. That commit has been superseded: the new code scopes the tooltip to a wrapper around only the path TextBlock, not the label. The concern is addressed; I've marked that thread resolved.

Overall assessment: The approach is correct and the targeted fix is sound. Background="Transparent" is correctly included so the padding area remains hit-testable. {x:Bind Path} inside the DataTemplate x:DataType="models:ShortcutFolder" is the right binding form for an in-tree element. No localization, service-wiring, or test changes are needed for this XAML-only change.

One non-blocking suggestion: Using Border instead of Grid as the single-child wrapper would be more idiomatic (see inline comment) — but this does not affect correctness or compilation.

No blocking issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ui Front-end changes, WinUI layouts, styling (e.g., Acrylic), or system tray updates.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant