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 DateRangeField #348

Merged
merged 1 commit into from
Dec 19, 2018
Merged

Add DateRangeField #348

merged 1 commit into from
Dec 19, 2018

Conversation

dferber90
Copy link
Contributor

Add DateRangeField

image

@dferber90 dferber90 force-pushed the df-add-date-range-field branch from 2564671 to 0d4923b Compare December 19, 2018 13:20
| Props | Type | Required | Values | Default | Description |
| ---------------------- | ------------------ | :------: | ---------------------------------- | ------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `id` | `string` | - | - | - | Used as HTML `id` property. An `id` is auto-generated when it is not specified. |
| `horizontalConstraint` | `object` | | `xs`, `s`, `m`, `l`, `xl`, `scale` | `scale` | Horizontal size limit of the input fields. |
Copy link
Contributor

Choose a reason for hiding this comment

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

let's make our lives easier? #325

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to do that in one go at the end instead of having to think about it for every component now. I want to tackle it once for all fields instead of ending up with a mix of some components only offering the values they support while others still offer all values.

It's also easier to communicate that we didn't do this at all so far and will tackle it once later instead of trying to communicate that we only did it for some and not for others and then consumers can never be sure which ones are supported and which ones aren't.

static propTypes = {
// DateRangeField
id: PropTypes.string,
horizontalConstraint: PropTypes.oneOf(['xs', 's', 'm', 'l', 'xl', 'scale']),
Copy link
Contributor

Choose a reason for hiding this comment

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

I would remove support for the unsupported one here but 🤷‍♂️

Copy link
Contributor

@montezume montezume left a comment

Choose a reason for hiding this comment

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

Works for me like this, if you want we can remove the unsupported constraints before though

@dferber90 dferber90 merged commit 3dcf6bf into master Dec 19, 2018
@dferber90 dferber90 deleted the df-add-date-range-field branch December 19, 2018 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
👨‍🎨 Status: UI/UX Review Requires review from design team 🚀 Type: New Feature Something new
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants