formkit / tempo

📆 Parse, format, manipulate, and internationalize dates and times in JavaScript and TypeScript.
https://tempo.formkit.com
MIT License
2.37k stars 33 forks source link

FeatureReuet: Support for the timezone token "ZZ" and changing the timezone format … #52

Closed Toru-Takagi closed 7 months ago

Toru-Takagi commented 7 months ago

close #46

I've implemented the feature you previously requested. I apologize for the inconvenience during your busy schedule, but I would appreciate it if you could check it. If there are any issues, I would like feedback. I will consider making corrections or closing the PR.

vercel[bot] commented 7 months ago

@Toru-Takagi is attempting to deploy a commit to the Formkit Team on Vercel.

A member of the Team first needs to authorize it.

vercel[bot] commented 7 months ago

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

Name Status Preview Comments Updated (UTC)
tempo-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 17, 2024 1:31pm
justin-schroeder commented 7 months ago

Thank you @Toru-Takagi. I think this is quite close — I think it might need some additional help on the parsing. Can you add some tests for using parse() with Z vs ZZ? That might help point out what needs to be updated on that side of things. Thank you!

Toru-Takagi commented 7 months ago

@justin-schroeder

Thank you for the review. 🙇 Now that I've understood a bit more, may I ask some questions for further clarification?

When running the following test with the current code, the result is similar to what's commented out.

    // first: Success
    expect(
      parse("1994-06-22T04:22:32+09:00", "YYYY-MM-DDTHH:mm:ssZ").toISOString()
    ).toBe("1994-06-21T19:22:32.000Z")
    // second: Success
    expect(
      parse("1994-06-22T04:22:32-0900", "YYYY-MM-DDTHH:mm:ssZ").toISOString()
    ).toBe("1994-06-22T13:22:32.000Z")
    // third: Fail
    expect(
      parse("1994-06-22T04:22:32-0900", "YYYY-MM-DDTHH:mm:ssZZ").toISOString()
    ).toBe("1994-06-22T13:22:32.000Z")

Is this the result you were expecting? first: Success, second: Fail, third: Success

or

first: Success, second: Success, third: Success

I apologize for bothering you while you're busy, but I'm looking forward to your response.

justin-schroeder commented 7 months ago

Correct @Toru-Takagi, my expectation of those tests would be: Success, Fail, Success

Toru-Takagi commented 7 months ago

@justin-schroeder Apologies for all the questions. Should passing [+-]HHmm as the second argument to applyOffset and removeOffset result in an error, or should it succeed?

Similarly, should passing [+-]HHmm when FormatStyle is full result in an error, or should it succeed?

justin-schroeder commented 7 months ago

@Toru-Takagi I think applyOffset should accept both [+-]HH:mm and [+-]HHmm.

Now that I think about it — with this change formats that output Z Should probably change to ZZ. For example:

formatStr({ time: 'full' }, 'en')
// h:mm:ss A Z

The above should probably output h:mm:ss A ZZ after this pull request.

Toru-Takagi commented 7 months ago

@justin-schroeder

I've made some corrections. Does it align with the intended implementation? 🙇

https://github.com/formkit/tempo/pull/52/commits/331d8966cf54ac17cb459704a30981c00f770fe3

justin-schroeder commented 7 months ago

This looks good @Toru-Takagi, merging this into the next release. I think I will tweak the time style so long outputs ZZ instead of Z but thats a minor change now. Great work!