Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Start selection from padding #5626

Closed
synchronos-t opened this issue Apr 28, 2020 · 6 comments · Fixed by #6343
Closed

Start selection from padding #5626

synchronos-t opened this issue Apr 28, 2020 · 6 comments · Fixed by #6343
Assignees
Labels
Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone

Comments

@synchronos-t
Copy link

Description of the new feature/enhancement

This is just a minor UX issue, but I'd intuitively expect that text selection is also possible if I start dragging with mouse from the padding (i.e. the margin between the content and window border). Were it possible, it would also make selection from the beginning of a line much easier.

In most of the text editors starting to drag from the left padding also selects full lines. That is maybe not needed in a terminal, but it's perhaps still something to consider, whether it'd be beneficial and expected. Most of those work so that the selection stays in the full line mode even if you drag onto the content area, but I also started to think whether it'd make sense in a terminal to only select full lines if the drag stays on the left padding, and select normally (last line is selected up to the mouse position) if the drag moves onto the content. In any case, this full-line selection from left padding is not as expected a functionality than the possibility to start normal selection from padding (any side of the padding).

@synchronos-t synchronos-t added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label Apr 28, 2020
@ghost ghost added Needs-Tag-Fix Doesn't match tag requirements Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Apr 28, 2020
@zadjii-msft
Copy link
Member

Wait I could have swore we fixed this with #4282/#4506 - @synchronos-t what version of the Terminal are you using?

@zadjii-msft zadjii-msft added Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. and removed Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. labels Apr 28, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Apr 28, 2020
@zadjii-msft zadjii-msft added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Apr 28, 2020
@DHowett-MSFT
Copy link
Contributor

So, this is part feature request part bug. There's the immediate bug ("I can't begin a selection if I start in the gutter"), and the request that gutter selection select the whole line like a word processor.

Just tried in 0.9 (before any selection changes), and the gutter doesn't allow selection there either. The linked changes are for unfocused terminals -- this is about padding in a focused one, presumably.

I'm going to take off NAF and triage this into "1.x"

@DHowett-MSFT DHowett-MSFT removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Apr 28, 2020
@DHowett-MSFT DHowett-MSFT added this to the Terminal v1.x milestone Apr 28, 2020
@DHowett-MSFT DHowett-MSFT added the Issue-Task It's a feature request, but it doesn't really need a major design. label Apr 28, 2020
@synchronos-t
Copy link
Author

Yes, sorry, should've provided the version number. Here they are, although I believe it doesn't matter that much any more. Yesterday I was using 0.11.1121.0 and now just updated to 0.11.1191.0. And yes, this was about selection in a focused terminal.

I thought about it yesterday whether to file this as a bug report or a feature request, and wasn't sure which one would be more suitable. After all, even the first part is just an usability enhancement, not something that is clearly broken. Anyway, I'd be happy with just the first part fixed; the second was more like a food for thought that came to mind while thinking about the selection from padding.

Anyway, good work with the Terminal so far!

@carlos-zamora
Copy link
Member

Huh. So, if you click on the space created by the padding (defined by SwapChainPanel.Margin()), you do not activate the PointerPressed handler. But this space in the UI belongs to the SwapChainPanel (because, where else would it go?). @DHowett would this be a bug to file on WinUI?

@DHowett
Copy link
Member

DHowett commented Jun 3, 2020

Margin applies outside a xaml element. Padding applies inside a xaml element. Consider: did we use the wrong one?

@ghost ghost added the In-PR This issue has a related PR label Jun 4, 2020
@ghost ghost closed this as completed in #6343 Jul 7, 2020
ghost pushed a commit that referenced this issue Jul 7, 2020
WinUI's `Margin` and `Padding` work very similarly. `Margin` distances
ourselves from our parent. Whereas `Padding` distances our children from
ourselves.

Terminal's `padding` setting is actually implemented by defining
`Margin` on the SwapChainPanel. This means that the "padding" that is
created is actually belongs to SwapChainPanel's parent: Grid (not to be
confused with its parent, "RootGrid").

When a user clicks on the padded area, input goes to Grid. But there's a
twist: you can't actually hit Grid. To be able to hit Grid, you can't
just set IsHitTestVisible. You need to set it's Visibility to Visible,
and it's Background to Transparent (not null) [2].

## Validation Steps Performed

- [X] Start a selection from the padding area
- [X] Click on a SearchBox if one is available
   - The SearchBox gets first dibs on the hit test so none gets through
     to the SwapChainPanel

## References
[1] https://docs.microsoft.com/en-us/uwp/api/windows.ui.xaml.uielement.ishittestvisible
[2] https://docs.microsoft.com/en-us/windows/uwp/xaml-platform/events-and-routed-events-overview#hit-testing-and-input-events

Closes #5626
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Jul 7, 2020
@ghost
Copy link

ghost commented Jul 22, 2020

🎉This issue was addressed in #6343, which has now been successfully released as Windows Terminal Preview v1.2.2022.0.:tada:

Handy links:

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants