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

Add clock click action settings #992

Merged
merged 14 commits into from
Jan 8, 2025
Merged

Conversation

xoascf
Copy link
Collaborator

@xoascf xoascf commented Dec 31, 2024

Clock Functionality Enhancements:

  • Removed the singleClick DispatcherTimer and its associated methods from Clock.xaml.cs, simplifying the clock's event handling (otherwise, delayed response for single clicking). [1] [2] [3]
  • Added support for various clock click actions (e.g., opening the Aero calendar, Modern calendar, Action Center for Windows 10/Notification Center for Windows 11) by updating the Clock_OnMouseLeftButtonDown method in Clock.xaml.cs.
  • Introduced a new ClockFlyoutLauncher class to handle the display of the Aero calendar flyout.

UI and Settings Adjustments:

  • Updated English.xaml to include new string resources for clock click actions and their values.
  • Modified PropertiesWindow.xaml to add a new combo box for selecting clock click actions in the settings UI.
  • Updated Settings.cs to include a new ClockClickOption enum and a corresponding property to store the user's clock click action preference. [1] [2]

Compatibility Improvements:

  • Added logic in PropertiesWindow.xaml.cs to list only available clock click actions for pre-Windows 10 systems, ensuring compatibility with previous versions of Windows (7, 8, 8.1). [1] [2]

Current Limitations:

  • If the user opens the Aero calendar while the taskbar is unlocked, the taskbar may inadvertently resize and/or change location.
  • If the user moves the mouse very quickly to open the Aero calendar, it might appear centered on the screen.
  • The Modern calendar is displayed for the primary screen only. (f78d9b4: using ManagedShell implementation).
  • The Modern calendar might flicker when opened while trying to move to the corner. ('').
  • The Modern calendar doesn't always open after clicking the clock (at least here for me on the latest Windows 10 LTSC). ('').
  • Some Modern flyouts may still float above RetroBar if the stock taskbar is larger than it.

@dremin dremin self-requested a review December 31, 2024 04:17
Copy link
Owner

@dremin dremin left a comment

Choose a reason for hiding this comment

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

This is really nice! Mostly some questions below.

RetroBar/PropertiesWindow.xaml.cs Outdated Show resolved Hide resolved
RetroBar/PropertiesWindow.xaml.cs Outdated Show resolved Hide resolved
Comment on lines 68 to 71
// Keep the flyout at least 3 by 3 pixels inside the working area
int newX = (int)Math.Max(taskbarScreen.WorkingArea.Left + 3, Math.Min(taskbarScreen.WorkingArea.Right - (clockFlyoutRect.Width) - 3, clockFlyoutRect.Left));
int newY = (int)Math.Max(taskbarScreen.WorkingArea.Top + 3, Math.Min(taskbarScreen.WorkingArea.Bottom - (clockFlyoutRect.Height) - 3, clockFlyoutRect.Top));
SetWindowPos(clockFlyoutHwnd, IntPtr.Zero, newX, newY, 0, 0, (int)(NoPosFlags));
Copy link
Owner

Choose a reason for hiding this comment

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

Why 3?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The last time I checked, it was the margin (spacing?) that separated the taskbar from the clock flyout in Windows 7, though I haven't checked if it applies to other screens with different scaling.

Copy link
Owner

Choose a reason for hiding this comment

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

Hmm, I just compared and my Win7 has 2px additional on the X and 1px additional on the Y, at least with Aero.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

After testing again, it appears that the correct value for both is 15 instead of 3.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just to found out that Vista does not use spacing (no floating flyout)...
VistaClockFlyoutNoSpacing

Maybe this should be customizable by the theme.

Copy link
Owner

Choose a reason for hiding this comment

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

Given that RetroBar targets emulating Vista and below, let's remove the margin for now.


className.Clear();
_ = GetClassName(hwnd, className, className.Capacity);
if (className.ToString() != "Windows.UI.Core.CoreWindow")
Copy link
Owner

Choose a reason for hiding this comment

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

Is there any way we can more uniquely check this? There are a lot of modern windows with this class, and I was able to get this code to move them, for example by using the keyboard to activate another flyout at the same time.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just found this Gist with C code that allows to "toggle the clock flyout in the Windows 10 taskbar on the monitor containing the mouse" (seems to be really complex, but it claims to be able to use other screens):
https://gist.github.com/valinet/bfc3e15d0d999cd3d2aab8d554955e6f

It seems that there's a program called "ExplorerPatcher", that allows to use the modern clock for the taskbar too: https://github.com/valinet/ExplorerPatcher

The Gist is also referenced here: valinet/ExplorerPatcher#67

Copy link
Owner

Choose a reason for hiding this comment

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

Overwriting memory in explorer.exe seems not great but that does look like a way to fix the multi-screen support. However, we would still need this loop to move the window to the proper edge, and I think there are some issues with OS version support:

  • With the Windows 11 taskbar, there is no window representing the clock button, so I don't think the code would work.
  • On my Windows 10 VM, the clock button class is TrayClockWClass rather than ClockButton. I wonder where ClockButton came from? I see TrayClockWClass on Windows 7 as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Developers seem to use a different method to invoke the flyout (ImmersiveFlyouts.c#L59-L145), no idea if that helps, probably as undocumented as the other ways to invoke the other clock flyouts.
EPClockFlyoutInterest

Copy link
Owner

Choose a reason for hiding this comment

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

That method looks much better!

Copy link
Owner

Choose a reason for hiding this comment

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

I have it working, but it is still non-functional in 24H2, due to the old taskbar code being removed from Windows. In light of this, I'm not sure we should pursue offering the modern calendar flyout option..

Copy link
Owner

@dremin dremin Jan 6, 2025

Choose a reason for hiding this comment

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

Good news, I have things working on 24H2 (well, even newer), and it also positions correctly using the API instead of needing to move the window. I'm working on a helper class for these flyouts in ManagedShell and once that is in we can use them.

Screenshot 2025-01-06 at 12 53 47 AM

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The implementation works as it should, except with RetroBar (?), maybe some problem in the work area, I don't think we should move the flyout as before.

2 rows (works fine):
w11clocktwo

1 row (it's floating, but it shouldn't be):
w11clockov

The same issue can be replicated on Windows 10 when number of rows is 1:
w10clockov

Why is the clock there, near the Quick Launch?
I usually do this with the controls to make sure the flyout opens in the right place, whether it is at either end, or in the middle, or slightly to the left or right.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Seems to affect Start menu invoked flyouts too:
floatinSearchFlyout

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah, that's a pre-existing issue with all of the flyouts. They won't overlap where the Explorer taskbar would be.

RetroBar/Utilities/ClockFlyoutLauncher.cs Outdated Show resolved Hide resolved
RetroBar/Utilities/ClockFlyoutLauncher.cs Outdated Show resolved Hide resolved
RetroBar/Utilities/Settings.cs Outdated Show resolved Hide resolved
RetroBar/Controls/Clock.xaml.cs Show resolved Hide resolved
RetroBar/Languages/English.xaml Outdated Show resolved Hide resolved
@dremin
Copy link
Owner

dremin commented Jan 1, 2025

If the user opens the Aero calendar while the taskbar is unlocked, the taskbar may inadvertently resize and/or change location.
If the user moves the mouse very quickly to open the Aero calendar, it might appear centered on the screen.

I wasn't able to reproduce either of these, how are you able to make this happen?

The Modern calendar doesn't always open after clicking the clock (at least here for me on the latest Windows 10 LTSC).

How unreliable is it? With this and the other issues I am wondering if we should hide the option for this for now.

@1280px
Copy link
Contributor

1280px commented Jan 1, 2025

New strings for Russian l10n (left Modern untranslated because I think it refers to Windows 10's Modern UI?), also update 1 older string for consistency, starting line 54:

    <s:String x:Key="middle_mouse_task_action">Действие по _средней кнопке мыши:</s:String>
    <x:Array x:Key="middle_mouse_task_action_values" Type="s:String">
        <s:String>Ничего не делать</s:String>
        <s:String>Открыть новое окно задачи</s:String>
        <s:String>Закрыть задачу</s:String>
    </x:Array>
    <s:String x:Key="clock_click_action">Действие по нажатию на _часы:</s:String>
    <x:Array x:Key="clock_click_action_values" Type="s:String">
        <s:String>Ничего не делать</s:String>
        <s:String>Открыть Aero-календарь</s:String>
        <s:String>Открыть Modern-календарь</s:String>
        <s:String>Открыть центр уведомлений</s:String>
    </x:Array>

Also, the "Open Modern calendar" option doesn't work for me on Windows 11 24H2 :с

@dremin
Copy link
Owner

dremin commented Jan 1, 2025

Also, the "Open Modern calendar" option doesn't work for me on Windows 11 24H2 :с

Ah, it doesn't work in my 24H2 VM either.

@xoascf
Copy link
Collaborator Author

xoascf commented Jan 1, 2025

Also, the "Open Modern calendar" option doesn't work for me on Windows 11 24H2 :с

Ah, it doesn't work in my 24H2 VM either.

Yeah, sorry, I haven't tested this on Windows 11 yet, so maybe that's why! 😅

Copy link
Owner

@dremin dremin left a comment

Choose a reason for hiding this comment

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

Per the inline comment, in the interest of getting this PR merged, I think we should hide the option to open the Modern flyout for now given the various issues and lack of support in 24H2. We can add it back in a separate PR afterwards if the issues can be resolved.

xoascf and others added 5 commits January 4, 2025 03:07
Override settings without overwriting them, allowing the user to keep his or her settings while the behavior matches that of the system.
So we can use and call the system flyouts via API.
Let's use the API to invoke the system flyouts.
Copy link
Owner

@dremin dremin left a comment

Choose a reason for hiding this comment

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

Thanks! 🎉

@xoascf
Copy link
Collaborator Author

xoascf commented Jan 9, 2025

New strings for Russian l10n (left Modern untranslated because I think it refers to Windows 10's Modern UI?), also update 1 older string for consistency, starting line 54:

    <s:String x:Key="middle_mouse_task_action">Действие по _средней кнопке мыши:</s:String>
    <x:Array x:Key="middle_mouse_task_action_values" Type="s:String">
        <s:String>Ничего не делать</s:String>
        <s:String>Открыть новое окно задачи</s:String>
        <s:String>Закрыть задачу</s:String>
    </x:Array>
    <s:String x:Key="clock_click_action">Действие по нажатию на _часы:</s:String>
    <x:Array x:Key="clock_click_action_values" Type="s:String">
        <s:String>Ничего не делать</s:String>
        <s:String>Открыть Aero-календарь</s:String>
        <s:String>Открыть Modern-календарь</s:String>
        <s:String>Открыть центр уведомлений</s:String>
    </x:Array>

Also, the "Open Modern calendar" option doesn't work for me on Windows 11 24H2 :с

@1280px, it would be nice to have those lines updated in a proper pull request.
By the way, you can check out the Modern calendar thing in the latest builds from GitHub Actions and see for yourself how it now works on Windows 11!

@xoascf
Copy link
Collaborator Author

xoascf commented Jan 9, 2025

If the user opens the Aero calendar while the taskbar is unlocked, the taskbar may inadvertently resize and/or change location.
If the user moves the mouse very quickly to open the Aero calendar, it might appear centered on the screen.

I wasn't able to reproduce either of these, how are you able to make this happen?

@dremin, just click on the clock to open the Aero calendar and move the cursor anywhere else (really quickly), the calendar may appear in the center, it may "lock" and allow the taskbar to drag it to any edge of the screen.

explorer_rb_drag_issue.mp4

@dremin
Copy link
Owner

dremin commented Jan 9, 2025

If the user opens the Aero calendar while the taskbar is unlocked, the taskbar may inadvertently resize and/or change location.
If the user moves the mouse very quickly to open the Aero calendar, it might appear centered on the screen.

I wasn't able to reproduce either of these, how are you able to make this happen?

@dremin, just click on the clock to open the Aero calendar and move the cursor anywhere else (really quickly), the calendar may appear in the center, it may "lock" and allow the taskbar to drag it to any edge of the screen.
explorer_rb_drag_issue.mp4

Ah, I see. I think setting e.Handled=true in Clock_OnMouseLeftButtonDown might fix this, does that help?

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