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

[Gallery][All Platform]ContentDialog - When clicking inside the dialog box, the 'OK' button highlighting disappears. #16358

Closed
sakshi173 opened this issue Apr 18, 2024 · 5 comments · Fixed by #16751
Closed

[Gallery][All Platform]ContentDialog - When clicking inside the dialog box, the 'OK' button highlighting disappears. #16358

sakshi173 opened this issue Apr 18, 2024 · 5 comments · Fixed by #16751
Assignees
@Youssef1313
Labels
area/canary 🐤 Categorizes an issue or PR as relevant to canary testing difficulty/tbd Categorizes an issue for which the difficulty level needs to be defined. kind/bug Something isn't working triage/untriaged Indicates an issue requires triaging or verification

Comments

@sakshi173
Copy link

Current behavior

Uno.Gallery.-.Google.Chrome.2024-04-18.14-35-51.mp4

Expected behavior

WinUI.Desktop.2024-04-18.14-39-40.mp4

How to reproduce it (as minimally and precisely as possible)

  1. Open Uno Gallery APP
  2. Tap Burger Menu on the left top right.
  3. Tap "ContentDialog" from the "Component" list.
  4. Click on 'Open Dialog' button for any option.
  5. The Dialog box should open.
  6. Now click anywhere in the opened dialog box.
  7. Notice that the 'OK' button highlighting disappears.

Workaround

No response

Works on UWP/WinUI

None

Environment

No response

NuGet package version(s)

No response

Affected platforms

WebAssembly, Android, iOS, Skia (GTK on Linux/macOS/Windows)

IDE

No response

IDE version

No response

Relevant plugins

No response

Anything else we need to know?

No response
@sakshi173 sakshi173 added kind/bug Something isn't working triage/untriaged Indicates an issue requires triaging or verification area/canary 🐤 Categorizes an issue or PR as relevant to canary testing difficulty/tbd Categorizes an issue for which the difficulty level needs to be defined. labels Apr 18, 2024
@sakshi173 sakshi173 changed the title [Gallery][All Platform]ContentDialog When clicking inside the dialog box, the 'OK' button highlighting disappears. [Gallery][All Platform]ContentDialog - When clicking inside the dialog box, the 'OK' button highlighting disappears. Apr 18, 2024
@agneszitte
Copy link
Contributor

agneszitte commented May 9, 2024 •
edited

@MartinZikmund, @kazo0, @morning4coffe-dev I think the "Expected Result" is not correct and the current behavior is the correct one no?

@MartinZikmund
Copy link
Member

As the expected video shows WinUI, I think it should be the correct behavior, as in our case clicking ContentDialog seems to unfocus the button, whereas on WinUI the button keeps the focus.

On the other hand (and that is a different issue) - I think this is Material style of dialogs while the tab says it should be fluent 🤔

@morning4coffe-dev
Copy link
Member

I think the implementation is different Material/Fluent. In Fluent (WinUI) the Expected behavior is the correct one while with Material you are able to deselect the primary button

Recording.2024-05-09.162009.mp4

@MartinZikmund
Copy link
Member

But as this is a style concern, I am not sure whether we keep the behavior separate in the two themes easily. On the other hand, the difference should not be a deal breaker, but we should still investigate, whether the unfocus should not be happening in Uno (as part of the popup handling)

@Xiaoy312 Xiaoy312 assigned Xiaoy312 and unassigned Xiaoy312 May 13, 2024
@Youssef1313
Copy link
Member

This happens because of

uno/src/Uno.UI/UI/Xaml/Internal/InputManager.Pointers.cs

Lines 192 to 205 in 057b269

// Uno specific: To ensure focus is properly lost when clicking "outside" the app's content,
// we set focus here. In the case of UWP, the focus is set to the root ScrollViewer instead,
// but Uno does not have it on all targets yet.
var focusedElement = _inputManager.ContentRoot.XamlRoot is { } xamlRoot
? FocusManager.GetFocusedElement(xamlRoot)
: FocusManager.GetFocusedElement();
if (!isHandled // so isAfterHandledUp is false!
&& _canUnFocusOnNextLeftPointerRelease
&& args.GetCurrentPoint(null).Properties.PointerUpdateKind is PointerUpdateKind.LeftButtonReleased
&& !PointerCapture.TryGet(args.Pointer, out _)
&& focusedElement is UIElement uiElement)
{
uiElement.Unfocus();
}

Ideally, this code should be removed once we have RootScrollViewer, and ScrollViewer itself would handle focus on pointer release, see:

image

In the above case when inside a popup, there will be no ancestor ScrollViewer, so that's why focus shouldn't change.

@Youssef1313 Youssef1313 mentioned this issue May 16, 2024
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Youssef1313 Youssef1313

Labels
area/canary 🐤 Categorizes an issue or PR as relevant to canary testing difficulty/tbd Categorizes an issue for which the difficulty level needs to be defined. kind/bug Something isn't working triage/untriaged Indicates an issue requires triaging or verification
Projects
None yet
Milestone
No milestone
6 participants
@MartinZikmund @Xiaoy312 @agneszitte @Youssef1313 @sakshi173 @morning4coffe-dev