Skip to content

Conversation

@mou-haz
Copy link
Contributor

@mou-haz mou-haz commented Jul 11, 2025

fix #148
focus visual now use control's corner radius.
improvement of focus target to various controls

@mou-haz mou-haz changed the title using control CornerRadius for focus visual [STYLE] Updated focus visuals Jul 11, 2025
Copy link
Member

@NotYoojun NotYoojun left a comment

Choose a reason for hiding this comment

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

  1. It seems focus visuals of Button controls have disappeared.
  2. It's not correct to simnply use a same value from the control. You should follow this instead:
image

@mou-haz
Copy link
Contributor Author

mou-haz commented Jul 19, 2025

Hey @NotYoojun , can u check if the new corner radius is correct ?
also, button focus visuals are working now.

@GID0317
Copy link
Contributor

GID0317 commented Jul 19, 2025

Hi, can you also take a look at the settings expander? it gets some cut issue bug
Settings Expander (Settings page example)
Current:
image
image
image

CommunityToolkit:
image
image
image

The rating controls focus on WinUI seems like doesn't use rounded corners:
Current:
image

WinUI:
image

The slider focus also seems like doesn't get effected by new rounded corners change:
Current:
image

WinUI:
image

@mou-haz
Copy link
Contributor Author

mou-haz commented Jul 24, 2025

Hey @GID0317 , can u take a look now ?
for the settingsexpander, the focus visual are being clipped by the scrollviewer even if i add adornerdecorator,
the only workaround i found is to increase the margins on the settingsexpander, so focus visuals are away from clipping.

@GID0317
Copy link
Contributor

GID0317 commented Jul 24, 2025

Hi, for the workaround resolves the clipping issue perhaps @NotYoojun has any suggestion for this?

For the main clipping i think moving padding from scroll viewer to SimpleStackPanel fix the issue for outer settings expander:
image

But the inner one is still affected like what mou-haz said:
image

Also, I noticed the expander non rounded point (expanded state) get effected by the change. can you please take a look mou-haz?
image
image
image

WinUI:
image
image
image

Some also seems not effected on gallery side for expander expand mode:
image

Calendar View and Picker:
Current:
image

WinUI:
image

TreeView seems like need some adjustment too to Match with WinUI:
Current:
image

WinUI
image

MenuBar and MenuFlyout Focus :
Current:
image
image
image

WinUI:
image
image
image

TabView:
Current:
image

WinUI:
image

Regarding the other issue you mentioned after testing, it looks great now

@mou-haz mou-haz force-pushed the fix_148 branch 2 times, most recently from e8f93c5 to 667d4e3 Compare July 24, 2025 22:06
@mou-haz
Copy link
Contributor Author

mou-haz commented Jul 24, 2025

@GID0317 , can u take a look now ? i think all mentioned controls except the datepicker are fine in this PR.

@GID0317
Copy link
Contributor

GID0317 commented Jul 25, 2025

Hi, after testing I think the issue was fixed now. Great work!

But unfortunately, i think some of it still need some adjustment. the latest commit change makes the "context menu, MenuBar
MenuFlyout" to bigger than before. i think the previous version size is already perfect (The radio menu items margin seems to be too):
Current:
Screenshot 2025-07-25 090851
image
image
image

Old Version:
image
image
image
image

I think the closer right size and focus for "MenuBar and MenuFlyout" was like the Combo box:
image

For the "context menu" i think the previous commit already good since it should be different from MenuBar and
MenuFlyout like in the WinUI:
Current:
Screenshot 2025-07-25 091657

Old Version:
image

WinUI:
image

The other one is it seems the selection indicator get overridden by the focus in this commit:
Current:
image

Old Version:
image

WinUI:
image

The last one is the Accent Button seems not effected by the rounded corners in the latest commit like what it should be
Current:
Screenshot 2025-07-25 090722

WinUI:
image

Could you take a look at this issue? Thank you

@mou-haz
Copy link
Contributor Author

mou-haz commented Jul 25, 2025

@GID0317 , they are fixed now

@GID0317
Copy link
Contributor

GID0317 commented Jul 25, 2025

Nice works, its overall look good now!

The one on the left is for the copy paste context menu, and its focus visual is still full on WinUI. It should have some distance from the borders, as shown in the image. This behavior is specific to the context menu and can be observed across the WinUI implementation
Current:
image
image

What the focus visual outline should it look like:
WinUI:
image
image

Explorer:
image
image

Photo:
image
image

@mou-haz
Copy link
Contributor Author

mou-haz commented Jul 25, 2025

@GID0317 context menu shoud be fixed now

@mou-haz mou-haz requested a review from NotYoojun July 25, 2025 16:15
@GID0317
Copy link
Contributor

GID0317 commented Jul 25, 2025

Sorry for another report. I think I missed this one, but hopefully it will be the last one for now. which is the combo box. it should be rounder like dropdown, but it seems like the current one still look like on the images. I’ve included a comparison with WinUI and the dropdown for reference.
Current:
image

WinUI:
image

Dropdown:
image

@mou-haz
Copy link
Contributor Author

mou-haz commented Jul 25, 2025

@GID0317 i think it's correct now.

@GID0317
Copy link
Contributor

GID0317 commented Jul 25, 2025

Hi, after testing, I think I found a strange bug. on the first time when we do not interact with the Combo box its working but if we are interacting with it the rounded corners revert back:

Screen.Recording.2025-07-26.025302.mp4

@mou-haz
Copy link
Contributor Author

mou-haz commented Jul 25, 2025

using the #269 with this should fix the problem.

@GID0317
Copy link
Contributor

GID0317 commented Jul 26, 2025

using the #269 with this should fix the problem.

Okay then, I think it's great now. Thanks for your hard work on this issue!

@mou-haz mou-haz marked this pull request as draft August 25, 2025 17:07
@mou-haz mou-haz marked this pull request as ready for review August 25, 2025 17:07
@Jack251970
Copy link
Contributor

Jack251970 commented Sep 16, 2025

❤️Thanks for your excellent work! It works well for me. And I think there is still something we can improve.

ListBox:

Unnecessary focus visual:

image

ListView:

Unnecessary focus visual:

image

DataGrid:

Unnecessary focus visual:

image

CalendarView:

No focus for circled items and unnecessary for rectangle items:

image image

TimePicker:

Unrounded focus visual:

image

@mou-haz
Copy link
Contributor Author

mou-haz commented Sep 18, 2025

@Jack251970 Thanks for your feedback and review.
The requested changes have been applied, can u please take a look now ?

timepicker focus visual

improved calender view focus visuals

assign focus visual target

 focus visual target for:
hyperlink button
radio button
button
command bar
menu items
toggle switch

improve focus visuals

toggle switch
command bar

updated corner radius

fix button focus visual

Fix FocusVisual clipped by scrollviewer

fix settingscard clipped

tree view focus visuals

improved focus visuals

radio menu items focus visuals
menu items focus visuals
tabitem focus visuals fix
treeview focus visuals
generalizing AdornerDecorator

context menu item focus visuals

combobox focus visuals
@Jack251970
Copy link
Contributor

Jack251970 commented Sep 18, 2025

@Jack251970 Thanks for your feedback and review. The requested changes have been applied, can u please take a look now ?

It works well for me, thanks for your work!❤️

There still exists one small bug: When CalendarView is initialized, it does not focus on today as WinUI3. (When another date is selected, it correctly focuses on that data as WinUI3)

WinUI3:

image

UI.WPF.Modern:

image

Thanks for your work again!

Copy link
Member

@NotYoojun NotYoojun left a comment

Choose a reason for hiding this comment

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

I agree with Ye, let's get these things fixed first.

@GID0317
Copy link
Contributor

GID0317 commented Sep 18, 2025

I can confirm this happen on my side too:

Recording.2025-09-18.163338.mp4

@Jack251970
Copy link
Contributor

Jack251970 commented Sep 18, 2025

@GID0317 For the second issue (From After I dig deeper again i think there are ...) as you described recently, I think we should move it out of this PR and create a separator PR for it. I feel like this PR should not include so much things for clean reviewing experience (And I wonder if you can move those contents to #356? There are too much contents in this PR)

I noticed that you have create an issue for it #356. Good👍

@GID0317
Copy link
Contributor

GID0317 commented Sep 18, 2025

Good idea! I think I'll move it to #148 instead of keeping it here, as it still relates to the previous change so no need to open another issue

edit: The #356 is another issue since I think even on hover focus navigate it shouldn't bugged like that and it's not related to the current issue

@mou-haz
Copy link
Contributor Author

mou-haz commented Sep 18, 2025

@Jack251970 Thanks for your review.
@Jack251970 @GID0317 @NotYoojun , the initial calendar view focus should be working correctly now.

@mou-haz mou-haz requested a review from NotYoojun September 18, 2025 16:31
@mou-haz
Copy link
Contributor Author

mou-haz commented Sep 18, 2025

@GID0317 can u see if it now works for #316 ?

@Jack251970
Copy link
Contributor

@Jack251970 Thanks for your review. @Jack251970 @GID0317 @NotYoojun , the initial calendar view focus should be working correctly now.

It works as expected. Thanks for your work!

@mou-haz
Copy link
Contributor Author

mou-haz commented Sep 19, 2025

@Jack251970 You are welcome.

@GID0317
Copy link
Contributor

GID0317 commented Sep 19, 2025

Thank you! I can confirm that the issue with #316 is resolved

@mou-haz
Copy link
Contributor Author

mou-haz commented Sep 19, 2025

@GID0317 You are welcome. Thanks for the review.

@Jack251970
Copy link
Contributor

Jack251970 commented Sep 20, 2025

@mou-haz I found one strange issue.

WinUI3:

Between these two elements, you can directly switch the focus.

image

UI.WPF.Modern:

Between these two elements, you need to press tab again to switch the focus.

image

@GID0317
Copy link
Contributor

GID0317 commented Sep 20, 2025

It seems to be related to my recently opened issue in #358, and it can also be reproduced on one of the current stable versions

fixes the problem that the hyperlink (even when textblock is hidden) was stealing the focus, giving unexpected focus switch results.
@mou-haz
Copy link
Contributor Author

mou-haz commented Sep 23, 2025

@Jack251970 @GID0317 I'll try to look into these.

@mou-haz
Copy link
Contributor Author

mou-haz commented Sep 24, 2025

Hey @Jack251970, can u see if the problem still happens ?

@Jack251970
Copy link
Contributor

Jack251970 commented Sep 24, 2025

Hey @Jack251970, can u see if the problem still happens ?

It works perfectly! Thanks for your work!

Copy link
Contributor

@Jack251970 Jack251970 left a comment

Choose a reason for hiding this comment

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

It looks very good to me!

@mou-haz
Copy link
Contributor Author

mou-haz commented Sep 24, 2025

Hey @Jack251970, can u see if the problem still happens ?

It works perfectly! Thanks for your work!

Thanks for the review. You are welcome.

@NotYoojun
Copy link
Member

As far as I've found, there's one major issue - you probably should not remove ui:ElevationBorders in SettingsControls, they are always essential when creating elevated surfaces (e.g. clickable)

After removing that SettingsControls get much more boring (just like how WpfUi looks).

I'll look into more details later. Sorry for the delay, I've got too much work to do in these months.

@mou-haz
Copy link
Contributor Author

mou-haz commented Nov 4, 2025

Hey @NotYoojun , thanks for the review.
The requested changes have been made. can you see if it is fine now ?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Selection Highlight Outline (Focus Visual) doesn't have rounded corner

4 participants