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

feat(new tool): Timezone Converter #1284

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

sharevb
Copy link
Contributor

@sharevb sharevb commented Sep 6, 2024

Fix #429

Copy link

vercel bot commented Sep 6, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
it-tools ✅ Ready (Inspect) Visit Preview Sep 21, 2024 0:42am

@steffenrapp
Copy link
Contributor

Hi again @sharevb This tool looks really powerful! I have tested it a bit and it seems the Timezones Date-Time Converter works fine also with multiple TZs selected. But I have some other findings/things that I don't really understand.

  • What's the matter of "Current Timezone Offset (min)"? It displays an offset based on the TZ but I can also edit the value in the field which doesn't change any calculation? The value can also be copied via the button but why? Maybe I absolutely don't get it or maybe it needs some explanation in the description.
  • Time is displayed in 24h format (maybe based on the browser settings?). Would be cool to be able to switch manually between 24h, am/pm and mixed like in https://www.worldtimebuddy.com
  • "Country to Timezones" and "Timezones to Countries" both don't do anything if I select something there. The output doesn't change.

Fixes Timezones <> Countries reactivity
Fix current timezone field to be readonlt
Added 24h format switch
@sharevb
Copy link
Contributor Author

sharevb commented Sep 13, 2024

Hi @steffenrapp, thanks for your reporting

  • What's the matter of "Current Timezone Offset (min)"? It displays an offset based on the TZ but I can also edit the value in the field which doesn't change any calculation? The value can also be copied via the button but why? Maybe I absolutely don't get it or maybe it needs some explanation in the description.

=> shows the selected timezone current timezeon offset (may be useful in case of DST) ; is now readonly

  • Time is displayed in 24h format (maybe based on the browser settings?). Would be cool to be able to switch manually between 24h, am/pm and mixed like in https://www.worldtimebuddy.com

=> made a switch

  • "Country to Timezones" and "Timezones to Countries" both don't do anything if I select something there. The output doesn't change.

=> should be ok now

@steffenrapp
Copy link
Contributor

Hi @sharevb thank you!
"Country to Timezones" and "Timezones to Countries" now work as expected.
I have two more findings:

  • 24h switch is nice but only works for the output. How about the input? Or does it pick the format from the browser?
  • "Current Timezone Offset" is read-only now but there is not enough space to display the content properly due to the 24h hour switch. Maybe you can improve that?
Bildschirmfoto 2024-09-13 um 15 20 10

@sharevb
Copy link
Contributor Author

sharevb commented Sep 14, 2024

Hi @steffenrapp

  • 24h switch is nice but only works for the output. How about the input? Or does it pick the format from the browser?

=> should be ok now

  • "Current Timezone Offset" is read-only now but there is not enough space to display the content properly due to the 24h hour switch. Maybe you can improve that?

=> should be ok now

@steffenrapp
Copy link
Contributor

Hi @sharevb 👋🏼
24h switch is fine now but the copy button for "Current Timezone Offset" is still overlapping the number. On smaller screens the switch is in a new line but the number is still not readable. Maybe remove the copy button or make the field bigger?
Bildschirmfoto 2024-09-14 um 12 05 21

@sharevb
Copy link
Contributor Author

sharevb commented Sep 14, 2024

Hi @steffenrapp, yes, removed copy icon as not so important (but overlap was not reproducable in chrome) or may be a bug of NaiveUI in your browser

@steffenrapp
Copy link
Contributor

Hi @sharevb thank you. Okay interesting ... in Safari the field is still a lot too small and doesn't resize according to the content. I think this field was displayed properly before the 24h switch was introduced 🤷🏻‍♂️
Bildschirmfoto 2024-09-14 um 12 42 12

@sharevb
Copy link
Contributor Author

sharevb commented Sep 14, 2024

Hi @steffenrapp, tried another type of layout, will see...cannot reproduce as I have no iOS

@steffenrapp
Copy link
Contributor

Hi @sharevb yes now I can read the content on iOS in Safari, too 👍🏼 just the space between the two lines is a little big I think but on a bigger screen it looks fine.
image

@sharevb
Copy link
Contributor Author

sharevb commented Sep 14, 2024

Hi @steffenrapp, probably related to the screen side and iOs, on chrome with simulated phone mode, spacing is ok and by the way, found no property on naiveui n-space for this

@steffenrapp
Copy link
Contributor

@sharevb Alright, so it looks good to me from functionality side 😃 but I can't do a code review unfortunately.

Copy link

sonarcloud bot commented Sep 21, 2024

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.

[NEW TOOL] Timezone Converter
2 participants