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 Documentation for Date and Time Picker #199

Merged
merged 1 commit into from Dec 10, 2021

Conversation

LukasBoll
Copy link
Collaborator

add Documentation for Date and Time Picker

closes #196

Signed-off-by: Lukas Boll lukas-bool@web.de

@sdirix sdirix requested a review from TheZoker September 8, 2021 07:09
Copy link
Collaborator

@TheZoker TheZoker left a comment

Choose a reason for hiding this comment

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

LGTM, only small notes

Thanks!

src/components/docs/date-time-picker.js Outdated Show resolved Hide resolved
src/components/docs/date-time-picker.js Outdated Show resolved Hide resolved
src/components/docs/date-time-picker.js Outdated Show resolved Hide resolved
src/components/docs/date-time-picker.js Outdated Show resolved Hide resolved
src/components/docs/date-time-picker.js Outdated Show resolved Hide resolved
</TableBody>
</Table>
</div>
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add a new line at the end of the file

src/components/docs/date-time-picker.js Outdated Show resolved Hide resolved
content/docs/date-time-picker.mdx Show resolved Hide resolved
@LukasBoll
Copy link
Collaborator Author

Thanks for the review, I applied your suggestions and updated the Pull request.

Copy link
Collaborator

@TheZoker TheZoker left a comment

Choose a reason for hiding this comment

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

LGTM, thanks 🎉

Copy link
Member

@sdirix sdirix left a comment

Choose a reason for hiding this comment

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

Already looks good!

We're mainly missing on how to customize the locale in each picker's modal. Here is the example in our example app: https://github.com/eclipsesource/jsonforms/blob/5da2f0d3afcecc9e2e663ec1238d08ad046dac80/packages/example/src/index.tsx#L44-L47

Please add a separate section where it is explained how to set the global dayjs locale. Ideally with a demo where you can switch between german and english locale like in the example above. Also mention that the dayjs locale customization is React Material only.

Otherwise I went into a lot of detailed comments for the first section. Please apply the feedback also to the remaining sections.

content/docs/date-time-picker.mdx Show resolved Hide resolved
content/docs/date-time-picker.mdx Outdated Show resolved Hide resolved
content/docs/date-time-picker.mdx Outdated Show resolved Hide resolved
content/docs/date-time-picker.mdx Outdated Show resolved Hide resolved
There are two ways to integrate the Time Picker. It can be embedded via the schema or the UI schema. The Time Picker supports a variety of options, that can be configured in the UI schema.

### Schema Based
An Input Field will be displayed as Time Picker by setting the format of the corresponding field to “time” in the schema.
Copy link
Member

Choose a reason for hiding this comment

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

There are some weird quotations marks here

src/components/docs/date-time-picker.js Outdated Show resolved Hide resolved
src/components/docs/date-time-picker.js Outdated Show resolved Hide resolved
src/components/docs/date-time-picker.js Outdated Show resolved Hide resolved
src/components/docs/date-time-picker.js Outdated Show resolved Hide resolved
src/components/docs/date-time-picker.js Outdated Show resolved Hide resolved
@LukasBoll LukasBoll force-pushed the documentDatePicker branch 3 times, most recently from aa7d5ec to a913a57 Compare September 22, 2021 21:15
<MenuItem value={'en'}>en</MenuItem>
<MenuItem value={'de'}>de</MenuItem>
</Select>
{reloadJsForms===true?<ReloadJsonForms data = {data}reload={()=>{setreloadJsForms(false)}}/>:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To change the language, it is not enough to rerender this component or to render JsonForms with different probs, because some components don’t get rerendered/updated. I removed Jsonforms from the DOM just to add it again, this way everything gets updated.

closes eclipsesource#196

Signed-off-by: Lukas Boll <lukas-bool@web.de>
Copy link
Member

@sdirix sdirix left a comment

Choose a reason for hiding this comment

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

LGTM.

@sdirix sdirix merged commit 07c73c5 into eclipsesource:master Dec 10, 2021
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.

Document the new date pickers
3 participants