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

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

Merged

Conversation

Toru-Takagi
Copy link
Contributor

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.

Copy link

vercel bot commented Apr 14, 2024

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

A member of the Team first needs to authorize it.

Copy link

vercel bot commented Apr 15, 2024

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
Copy link
Member

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
Copy link
Contributor Author

@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
Copy link
Member

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

@Toru-Takagi
Copy link
Contributor Author

@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
Copy link
Member

@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
Copy link
Contributor Author

@justin-schroeder

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

331d896

@justin-schroeder justin-schroeder changed the base branch from main to release/0.1.0 April 17, 2024 13:35
@justin-schroeder justin-schroeder merged commit d42a114 into formkit:release/0.1.0 Apr 17, 2024
2 checks passed
@justin-schroeder
Copy link
Member

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!

justin-schroeder added a commit that referenced this pull request Apr 17, 2024
* feat: diff functions

* differenceInMilliseconds

* created the other constant difference functions, but will need to add the option 'ignoreTime' for day/week version

* created the other constant difference functions

* completing tests

* adding differenceInX exports in index

* make sure that rounding doesn't give -0

* added difference In Months & years

* shortened the all difference functions to diffX
moved using monthDays at diffMonths after `if (ld < rd)`

* raname type DifferenceRoundingMethod -> DiffRoundingMethod

* docs: adds diffs to docs

* chore: bumps tests to latest actions

* feat: existing "Z" token is now "ZZ" adds "Z" token style (#52)

* Support for the timezone token "ZZ" and changing the timezone format style.

* Fix timezone offset parsing and applyOffset function

* chore: adjusts time format < full to be ZZ

---------

Co-authored-by: WilcoSp <17604138+WilcoSp@users.noreply.github.com>
Co-authored-by: Kouta Motegi <toru.takagi.engineer@gmail.com>
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.

FeatureReuet: Support for timezone format +-HH:mm in format token.
2 participants