-
Notifications
You must be signed in to change notification settings - Fork 274
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 theme-independent clock format customization #754
Conversation
Also, moved the "Notification area" `ContentControl` and its `Border` into the resource dictionary as a `ControlTemplate` (using the key `"NotificationAreaPreview"`) so it can be reused.
I just realized, to my indescribable disappointment, that I accidentally deleted the new If there are any other changes I need to make, I will restore the controls along with those changes as a single commit. Otherwise, I'll restore the controls as their own commit. Another disaster: I mentioned "an unusual bug" that's fixed by resetting the culture when changing themes - the bug was that the Edit: Another thing I only just noticed - I mixed up some of the semicolons and commas in the "What the notations mean" text. I'll fix that as well pending a review. |
Thanks for working on this! I'll be taking a closer look after I get the latest release wrapped up. One thought that came to mind is that this should probably always override the theme clock format (when enabled), since otherwise it would be very inconsistent for vertical taskbars, which most themes set a clock format for. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies for the delay! This will need merge conflicts resolved before merging, and I also left some additional comments.
} | ||
break; | ||
|
||
case "Theme": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does Theme
need to be here? I don't believe it will influence SetCurrentCulture
, and it doesn't seem to fix the odd Properties preview bug you mentioned in your comment.
@@ -57,6 +58,14 @@ | |||
<s:String x:Key="debug_logging">Enable debug logging</s:String> | |||
<s:String x:Key="ok_dialog">OK</s:String> | |||
|
|||
<s:String x:Key="clock_appearance">Clock appearance</s:String> | |||
<s:String x:Key="clock_format_info" xml:space="preserve">What the notations mean: h = hour; m = minute, s = second, tt = AM/PM d = day; M = month; yy, yyyy = year ddd, dddd = weekday; MMM, MMMM = month name h/H = 12/24 hour hh, mm, ss, dd, MM = display leading zero h, m, s, d, M = do not display leading zero \n = split clock into multiple lines (not recommended on all themes) Use "\" in front of other notations to use their actual characters</s:String> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems a bit gnarly to localize. Given that we are using standard format specifiers, what are your thoughts regarding instead providing a link to this table which also includes examples and descriptions of all specifiers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -57,6 +58,14 @@ | |||
<s:String x:Key="debug_logging">Enable debug logging</s:String> | |||
<s:String x:Key="ok_dialog">OK</s:String> | |||
|
|||
<s:String x:Key="clock_appearance">Clock appearance</s:String> | |||
<s:String x:Key="clock_format_info" xml:space="preserve">What the notations mean: h = hour; m = minute, s = second, tt = AM/PM d = day; M = month; yy, yyyy = year ddd, dddd = weekday; MMM, MMMM = month name h/H = 12/24 hour hh, mm, ss, dd, MM = display leading zero h, m, s, d, M = do not display leading zero \n = split clock into multiple lines (not recommended on all themes) Use "\" in front of other notations to use their actual characters</s:String> | |||
<s:String x:Key="clock_format_warning" xml:space="preserve">Note: Custom clock formatting may not be applied if your selected theme specifies its own format.</s:String> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of putting a manual break in here, we can add padding/margin to the TextBlock to limit its width.
<GroupBox Header="{DynamicResource clock_appearance}" | ||
Margin="0,0,0,10"> | ||
<StackPanel Orientation="Vertical"> | ||
<ContentControl Template="{StaticResource NotificationAreaPreview}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bug with the preview seems to happen because the clock control has no children at the time the Loaded
event fires, so the ConverterCulture doesn't get set. I'm not sure why it is happening in this specific case.
IMO this should be fixed before the changes can be merged. There is an alternative, we could remove the extra preview, but that would make the Clock tab a bit sparse. Together with replacing the explanatory string with a link, the clock format section could fit on the Advanced tab instead.
Hello! |
this needs to be added |
wait what did do |
Any progress on this PR status? |
The review comments are not addressed. I may need to make the changes. |
This comment was marked as off-topic.
This comment was marked as off-topic.
I've decided to close this without merging for a few reasons:
|
clockformattab_demo2_cropped.mp4
Fixes #217, #300, #477, #671, #726
Partially fixes #110, #445, #508
Changes
Clock
tab to the settings dialog:\n
This works by simply overriding
CultureInfo.DateTimeFormat
'sShortTimePattern
,AMDesignator
, andPMDesignator
properties inClock.SetCurrentCulture()
with our custom values. Editing any of the formatting settings (or changing the theme, to fix an unusual bug) resets the culture, so your changes are reflected in real time.Also, I turned the "Notification area" preview into a
ControlTemplate
and moved it into the resource dictionary (using the key"NotificationAreaPreview"
) so I could reuse it. I don't know if this is how you're supposed to do it.Minor issues
\n
so the literal sequence "\n" cannot appear on the clock (unless you put it in the designators). I apologize for the thousands of issues that will surely be opened regarding this.Irrelevant notes
h:mm:ss tt | ddd, MMM d, yyyy
anda.m.
/p.m.
as the default format and designators to make it clear to the user how far they can take the formatting before they actually read the text at the bottom. I didn't include\n
however, as multi-line clocks don't look good on themes with harsh gradients or limited clock space (like the default theme).Languages/English.xaml
really, really wide.Show the clock
checkbox logically fits on both tabs, but I didn't want to disturb the nostalgic sanctity of its original placement, so I duplicated it. Let me know if that's okay.