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

Headless CMS - "multipleValues" renderer for Date/Time field #973

Merged
merged 8 commits into from Jun 12, 2020

Conversation

Ashu96
Copy link
Contributor

@Ashu96 Ashu96 commented Jun 9, 2020

closes #914

Related Issue

#914
Headless CMS - "multipleValues" renderer for Date/Time field

Your solution

Use DynamicListMultipleValues component

How Has This Been Tested?

Manually, using the Admin app

Screenshots:

https://www.loom.com/share/ed8b97db76e8426ba86af1aa70e4572f

@Ashu96 Ashu96 requested a review from adrians5j June 9, 2020 07:41
Copy link
Member

@adrians5j adrians5j left a comment

Choose a reason for hiding this comment

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

Let's try adjusting this button a bit:
image

P.S. The date/time icons are not vertically centered as well.

replace `ButtonDefault` with `IconButton`
@Ashu96 Ashu96 requested a review from adrians5j June 10, 2020 14:59
Copy link
Member

@adrians5j adrians5j left a comment

Choose a reason for hiding this comment

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

Please take a look at these minor things. Nothing critical.

import React from "react";
import {css} from "emotion";
import {Cell} from "@webiny/ui/Grid";
import {IconButton} from "@webiny/ui/Button";
Copy link
Member

Choose a reason for hiding this comment

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

Check prettier.

<IconButton className={deleteIconStyles} onClick={trailingIcon.onClick} icon={trailingIcon.icon} />
</Cell>
)
}
Copy link
Member

Choose a reason for hiding this comment

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

Check this as well.


const plugin: CmsEditorFieldRendererPlugin = {
type: "cms-editor-field-renderer",
name: "cms-editor-field-renderer-date-time",
Copy link
Member

Choose a reason for hiding this comment

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

While you're at it, check packages/app-headless-cms/src/admin/plugins/fields/dateTime.tsx:12, this one has dateTime in the name. Ideally we'd use kebab case here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, you opened this file in a different branch

@@ -3,16 +3,25 @@ import { I18NValue } from "@webiny/app-i18n/components";
import { CmsEditorField } from "@webiny/app-headless-cms/types";
import { BindComponentRenderProp } from "@webiny/form";
import { Input as UiInput } from "@webiny/ui/Input";
import {ReactNode} from "react";
Copy link
Member

Choose a reason for hiding this comment

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

Prettier.

@Ashu96 Ashu96 requested a review from adrians5j June 11, 2020 15:15
@adrians5j adrians5j merged commit acb34bf into develop Jun 12, 2020
@Pavel910 Pavel910 deleted the feat/914 branch June 16, 2020 16:40
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.

Headless CMS - "multipleValues" renderer for Date/Time field
2 participants