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

Support named timezones #2447

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

cleishm
Copy link

@cleishm cleishm commented Mar 16, 2025

Instead of just UTC+/UTC- offsets, support named timezones. These will adjust for DST correctly.

Instead of just UTC+/UTC- offsets, support named timezones. These will
adjust for DST correctly.
@exelban
Copy link
Owner

exelban commented Mar 16, 2025

Hi. Basically LGTM. I would like to merge that now, but before one feature is missed - support of current times. It means that if I merge that version it will remove all clocks the user has. I see 2 options: you can add that or I will merge that next week and will make some workaround.

PS: also the select box needs to be adjusted to correctly fit the name:

Zrzut ekranu 2025-03-16 o 11 31 08

@cleishm
Copy link
Author

cleishm commented Mar 16, 2025

I think I've resolved those concerns, although I'm not especially familiar with the UI frameworks. I think that control would also be improved if it was replaced with a text field using autocomplete and validation (rather than a very, very long list), but that seems a little too ambitious for now.

@@ -22,7 +22,7 @@ public struct Popup_c_s {
}

public struct Settings_c_s {
public let width: CGFloat = 540
public let width: CGFloat = 684
Copy link
Owner

@exelban exelban Mar 23, 2025

Choose a reason for hiding this comment

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

why did you change the settings window width?

Copy link
Author

Choose a reason for hiding this comment

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

Widening the Timezone column made the Name column unacceptably small, as the table is sized to fit the settings window. Increasing the overall width fixed that. But perhaps there is a better approach?

Copy link
Owner

@exelban exelban Mar 23, 2025

Choose a reason for hiding this comment

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

if I will increase the size of the settings window for such a small feature the width will be 10000+. So it's not acceptable at all.

  • scrollable table
  • resizing another tabs
  • etc

but the best solution will be just crop the text inside the select with ...

Copy link
Author

Choose a reason for hiding this comment

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

Whatever you think best. Just be aware that timezones are most significant on the right hand side, so truncating may make them ambiguous. Maybe a text field with autocomplete (and validation) for entry would be better, then you can truncate to the left when displaying.

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.

2 participants