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

Add datetime field #552

Merged
merged 9 commits into from
Aug 22, 2023
Merged

Add datetime field #552

merged 9 commits into from
Aug 22, 2023

Conversation

emilienbidet
Copy link
Contributor

Hello Thinkmill and keystatic team 👋🏻

CleanShot 2023-08-18 at 13 41 27@2x

Following #444

I need the datetime field for my personal project so I just did it.
Thank you open source, this is my first real contribution to keystatic, so glad to work on this project.

Added dateTime Field and Documentation

Changes Made

  1. Created a new field named datetime to handle DateTime input using the <input type="datetime-local"> form field.
  2. Implemented the DatetimeFieldInput component for capturing Datetime input.
  3. Added the validateDateTime function to handle DateTime validation.
  4. Created a new demo component named DatetimeFieldDemo to showcase the usage of the datetime field.
  5. Updated the documentation to include details about the datetime field, its usage, and type signature.

Please review the changes made and let me know if any further adjustments are needed.

@changeset-bot
Copy link

changeset-bot bot commented Aug 18, 2023

🦋 Changeset detected

Latest commit: 540088e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 8 packages
Name Type
@keystatic/core Patch
keystatic-docs Patch
@example/astro Patch
@example/localization Patch
@example/next-app Patch
@keystatic/remix-test-app Patch
@keystatic/templates-astro Patch
@keystatic/templates-nextjs Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Aug 18, 2023

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

Name Status Preview Comments Updated (UTC)
keystar-ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 22, 2023 8:44am
keystatic ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 22, 2023 8:44am
keystatic-site ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 22, 2023 8:44am

@JedWatson
Copy link
Member

Hey @emilienbidet!

Thanks for the PR. It's late Friday evening here in Australia so we probably won't land this before early next week, but at a glance it looks good. Will get you a proper review soon!

@emilienbidet
Copy link
Contributor Author

Hi @JedWatson !

Thank you for your answer! Okay, have a nice weekend then!
I wait for your review.

FYI : I didn't handle timezone, I went straight to basic.

Thank you very much everyone,
Have fun

@emilienbidet
Copy link
Contributor Author

I'm wondering... Why for keystatic date field we don't use the DateField from the design-system?

Another implementation of this datetime field would be to just to update the date field to make it use the DateField from the design-system and add the granularity props as a field customisation properties like this :

datetime: fields.date({
  label: 'Date and time',
  description: 'The date and time of the event',
  granularity: 'day' | 'hour' | 'minute' | 'second',
  validation: {
    isRequired: true,
    min: '2023-08-15T00:00Z',
    max: '2023-08-20T00:00Z',
  },
}),

I prefer this approach because it match to the design-system and let the dev choose what type of date he wants.

@JedWatson
Copy link
Member

Hey @emilienbidet!

We took a look at this, and I also had a chat with @jossmac about adding Time support to the DateField in the design system.

We'd like to expand on the design system component so it can handle selecting times as well, but it's not going to happen quickly, and it should be fine to update the UI for this field in the future when the design system component is ready; the native type is less consistent but will get things unblocked and it wouldn't be a breaking change to switch them out.

In terms of having one field or two, we're inclined to go with two, because we've found it easier to manage each field type having a distinct data type that it serialises to; e.g we're planning to improve on slugs in the near future and one of the things that we want is to be able to encode date / datetime field values into them to make filesystem sorting easier. In that case, you'd handle the value differently, and having distinct field types makes that easier than one field that has a variable type.

So we're happy to go with this implementation for now!

Copy link
Member

@JedWatson JedWatson left a comment

Choose a reason for hiding this comment

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

Great PR, thanks @emilienbidet!

Very happy to have you contributing to Keystatic ❤️

@JedWatson JedWatson merged commit c5407cc into Thinkmill:main Aug 22, 2023
9 checks passed
@emmatown emmatown mentioned this pull request Sep 25, 2023
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.

3 participants