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.

Sign up for GitHub

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

Jump to bottom

[SizerBase] Orientation and Cursor not set correctly #388

Closed
michael-hawker opened this issue Mar 8, 2023 · 1 comment · Fixed by CommunityToolkit/Windows#26
Closed

[SizerBase] Orientation and Cursor not set correctly #388

michael-hawker opened this issue Mar 8, 2023 · 1 comment · Fixed by CommunityToolkit/Windows#26
Labels
bug 🐛 Something isn't working experiment 🧪 Used to track issues that are experiments (or their linked discussions) issue 🧪 Used for tracking an Issue/Bug/Feature of an open Experiment/Component

Comments

@michael-hawker
Copy link
Member

michael-hawker commented Mar 8, 2023 •
edited
Loading

Parent Experiment: #101

Figured out what was going on with the cursor not properly updating when the Orientation is Horizontal (like in the sample) - regression from current Toolkit version.

It's this code here:

Labs-Windows/labs/SizerBase/src/SizerBase.Properties.cs

Lines 98 to 110 in 9045236

var cursor = gripper.ReadLocalValue(CursorProperty);
if (cursor == DependencyProperty.UnsetValue || cursor == null)
{
cursor = cursorToUse;
// On UWP, we use the extension in XAML to control this behavior,
// so we'll update it here (and maintain binding).
// We'll keep it in-sync to maintain behavior for WinUI 3 as well.
gripper.SetValue(CursorProperty, cursor);
// We return here, as the Cursor will trigger this function again anyway to set for WinUI 3
return;
}

Since we set the value of the dependency property, when the initial pass run on load sets it to SizeWestEast. Then the detection logic runs and Orientation is changed to Horizontal, but the set gets ignored the 2nd time as the prop has been set...

Think this was abstracted in the old code as the inner class held that info.

Here, I think we need to just have the private value tracking what the cursor should be and leave the Cursor property for overrides? Will think about it more and get back to it as a separate fix.

Effects UWP and WASM (basically WinUI 2), Windows App SDK uses a different setup, so I think is uneffected.

Originally posted by @michael-hawker in #101 (comment)

(Will have to create a new issue template for experiment bugs...)
@michael-hawker michael-hawker added experiment 🧪 Used to track issues that are experiments (or their linked discussions) bug 🐛 Something isn't working issue 🧪 Used for tracking an Issue/Bug/Feature of an open Experiment/Component labels Mar 8, 2023
@michael-hawker michael-hawker mentioned this issue Mar 8, 2023
Closed
@michael-hawker
Copy link
Member Author

michael-hawker commented Apr 14, 2023 •
edited
Loading

Huh, noticing the same behavior for GridSplitter where it's not setting the NW cursor on the horizontal bar for WASDK as well... looking at this again now...

Ah, different code, but same root cause, the initial pass runs and sets the ProtectedCursor and then I don't think sets it again as it's already set when the Orientation property is updated.

michael-hawker added a commit to michael-hawker/Labs-Windows that referenced this issue Apr 15, 2023
@michael-hawker
Fixes CommunityToolkit#388 SizerBase Orientation Property
9261a43 
Removes Mouse Extension from XAML Template and does in code-behind as to remove the need to set the Cursor property directly via binding
By setting the Cursor DependencyProperty we were losing our detection mechanism, now if it's set it's an explicit override, otherwise we use our logic based on Orientation property
Also, switches to new CommunityToolkit.*.Extensions Dependency vs. old copies of Tree helpers
Fixes some doc typos
Bumps version, tested on UWP, WASDK, and Uno.UI/WASM
michael-hawker added a commit to michael-hawker/Labs-Windows that referenced this issue Apr 15, 2023
@michael-hawker
Fixes CommunityToolkit#388 SizerBase Orientation Property
1035ff9 
Removes Mouse Extension from XAML Template and does in code-behind as to remove the need to set the Cursor property directly via binding
By setting the Cursor DependencyProperty we were losing our detection mechanism, now if it's set it's an explicit override, otherwise we use our logic based on Orientation property
Also, switches to new CommunityToolkit.*.Extensions Dependency vs. old copies of Tree helpers
Fixes some doc typos
Bumps version, tested on UWP, WASDK, and Uno.UI/WASM
michael-hawker added a commit to michael-hawker/Labs-Windows that referenced this issue Apr 15, 2023
@michael-hawker
Fixes CommunityToolkit#388 SizerBase Orientation Property
d2ecf62 
Removes Mouse Extension from XAML Template and does in code-behind as to remove the need to set the Cursor property directly via binding
By setting the Cursor DependencyProperty we were losing our detection mechanism, now if it's set it's an explicit override, otherwise we use our logic based on Orientation property
Also, switches to new CommunityToolkit.*.Extensions Dependency vs. old copies of Tree helpers
Fixes some doc typos
Bumps version, tested on UWP, WASDK, and Uno.UI/WASM
michael-hawker added a commit to michael-hawker/Labs-Windows that referenced this issue Apr 15, 2023
@michael-hawker
Fixes CommunityToolkit#388 SizerBase Orientation Property
1e36779 
Removes Mouse Extension from XAML Template and does in code-behind as to remove the need to set the Cursor property directly via binding
By setting the Cursor DependencyProperty we were losing our detection mechanism, now if it's set it's an explicit override, otherwise we use our logic based on Orientation property
Also, switches to new CommunityToolkit.*.Extensions Dependency vs. old copies of Tree helpers
Fixes some doc typos
Bumps version, tested on UWP, WASDK, and Uno.UI/WASM
This was referenced Apr 17, 2023
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug 🐛 Something isn't working experiment 🧪 Used to track issues that are experiments (or their linked discussions) issue 🧪 Used for tracking an Issue/Bug/Feature of an open Experiment/Component
Projects
Toolkit 8.x
Status: ✅ Done
Milestone
No milestone
1 participant
@michael-hawker