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

[Preview 4] Inconsistent Cursor Behavior with GridSplitter in WinUI3 Desktop #4201

Closed
eleanorleffler opened this issue Feb 16, 2021 · 9 comments
Labels
product-winui3 WinUI 3 issues team-Rendering Issue for the Rendering team version-winui3preview4 WinUI 3 Preview 4 issues wct

Comments

@eleanorleffler
Copy link

Describe the bug

In WinUI3 Preview4, the cursor does not change to the Resize cursor icon on the hover over before adjusting the GridSplitter as it does in UWP.

Steps to reproduce the bug

  1. Clone the WinUI3 Preview 4 Problems GridSplitter repository.
  2. Go to the GridSplitterWinUIPreview4 folder.
  3. Open the GridSplitterWinUIPreview4 solution in Visual Studio 2019 Preview.
  4. Build and run with Debug x64.
  5. Hover over one of the two GridSplitters (Horizontal or Vertical). You should see the cursor icon does not change to the Resize icon.

Expected behavior

We expect the cursor icon to change to the Resize icon when hovering over the GridSplitter.

Screenshots

ExpectedBehavior

Screenshot#1 - Expected Behavior

CurrentBehavior

Screenshot#2 - Current Behavior

Version Info

NuGet package version:

[Microsoft.Toolkit.Uwp.UI.Controls 8.0.0-preview4]
[Microsoft.WinUI 3.0.0-preview4.210210.4]

Targeting:
Target: Universal Windows
Target version: Windows 10, version 1809 (10.0; Build 17763)
Min version: Windows 10, version 1803 (10.0; Build 17134)

Windows app type:

UWP Win32
Yes
Windows 10 version Saw the problem?
Insider Build (xxxxx)
May 2020 Update (19041)
November 2019 Update (18363)
May 2019 Update (18362)
October 2018 Update (17763) Yes
April 2018 Update (17134)
Fall Creators Update (16299)
Creators Update (15063)
Device form factor Saw the problem?
Desktop Yes
Xbox
Surface Hub
IoT

Additional context

@ghost ghost added the needs-triage Issue needs to be triaged by the area owners label Feb 16, 2021
@StephenLPeters StephenLPeters added version-winui3preview4 WinUI 3 Preview 4 issues product-winui3 WinUI 3 issues and removed needs-triage Issue needs to be triaged by the area owners labels Feb 17, 2021
@StephenLPeters
Copy link
Contributor

@michael-hawker FYI, Does GridSplitter use CoreWindow? (API's with CurrentView)

@michael-hawker
Copy link
Collaborator

@StephenLPeters Yes, this is because it was trying to modify the PointerCursor with a CoreCursor. We haven't properly updated this code yet in the Toolkit.

We haven't evaluated if we have a workaround or a proper fix yet, as the protected Cursor property on UIElement hasn't been exposed as a public property on FrameworkElement yet (like it was in WPF). We may look at a workaround for 0.5 as we prepare for that release of the Toolkit on top of that in March. Here's the internal issue tracking the Cursor property being exposed, but I'm not sure when it's scheduled to be implemented.

FYI @azchohfi I believe you're aware of this issue, but we don't have an issue tracking this in our repo currently for GridSplitter specifically, eh? I'll add it to our known issues list at CommunityToolkit/WindowsCommunityToolkit#3295

@azchohfi
Copy link
Contributor

@michael-hawker Yeah, that should be enough., but we need to figure out how to fix this. And yes, I was aware of the issue.

@michael-hawker
Copy link
Collaborator

Based on how GridSplitter is currently implemented, it has issues with how the Hover is implemented since we don't have access to set the Cursor property. There's an issue tracking adding Cursor to FrameworkElement like it was in WPF. @codendone when is that currently scheduled to be exposed?

We may refactor this control in the 7.1 version of the Toolkit which slipped from 7.0. Though we normally don't ship breaking changes, so we have to understand the implications of doing that work in a minor release. The new implementation should be simpler and may be able to workaround this issue. Otherwise, we don't have plans currently for GridSplitter specifically to try and work-around this in the meantime as that'd be just as much work as our eventual planned refactor.

#4834 did also call out the general need to set the cursor for a window, which is what the Cursor property should provide as well, but there's not an issue on GitHub tracking that yet?

@jschwizer99
Copy link

@michael-hawker There was a discussion in #4682 (comment) on an accessible Cursor. In the answer #4682 (comment) I was redirected to a "spec" for a public Cursor Property https://github.com/microsoft/microsoft-ui-xaml-specs/blob/master/active/UIElement/ElementCursor.md. That is the up-to-date information that is publicly accessible. I'm keen to hear more on this topic if you find more information.

@codendone
Copy link
Contributor

@michael-hawker As @jschwizer99 said, I think GridSplitter should be able to use the UIElement.ProtectedCursor property. This is a protected property, but GridSplitter is a subclass so it has access.

We plan to eventually have a public UIElement.Cursor property as well, but don't have a timeframe for that yet. It is not planned for Project Reunion 1.0. But I think this should not be a blocker for GridSplitter.

@michael-hawker
Copy link
Collaborator

@codendone the current version of GridSplitter has a 'wrapper' class that provides the hover functionality which is outside the base class (I don't know why it was done this way initially, but we later added helpers for UWP to make it easier to change the cursor). In either case, this contained helper class doesn't have access to the ProtectedCursor of its parent currently. Thus, why this doesn't work properly currently.

We've been wanting to re-write GridSplitter for a while, we have a pending start of that work that didn't make it in 7.0. In that version we'll have removed this helper class, so it should resolve the problem for this particular scenario for now.

Still concerned about other scenarios though, as our extensions to provide developers ways to change the cursor on any arbitrary element will still be broken, as is an easy way to change the cursor globally at the app-level, correct?

@codendone
Copy link
Contributor

@michael-hawker What is GridSplitter a wrapper class for? If it is a wrapper for another subclass, that subclass could create its own public property or accessor functions.

The ProtectedCursor property enables many scenarios, including ones which weren't available in system XAML. We do still need an equivalent of CoreWindow.PointerCursor, as well as the eventual UIElement.Cursor, but don't have a timeline for these. Having a subclass panel at the root of the window is a potential workaround today for the app-level cursor scenario.

@codendone codendone added the team-Rendering Issue for the Rendering team label Jul 28, 2021
@codendone
Copy link
Contributor

Collapsing this down to one issue, and I'm choosing to keep #4834 open, since that has the more recent sample.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product-winui3 WinUI 3 issues team-Rendering Issue for the Rendering team version-winui3preview4 WinUI 3 Preview 4 issues wct
Projects
None yet
Development

No branches or pull requests

6 participants