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

feat(FluidDatePicker): implement FluidDatePicker #12247

Merged
merged 20 commits into from
Nov 8, 2022

Conversation

tw15egan
Copy link
Member

@tw15egan tw15egan commented Oct 5, 2022

Refs #12125
Closes #12403

Adds in unstable__FluidDatePicker and unstable__FluidDatePickerInput

Changelog

New

  • unstable__FluidDatePicker component
  • Styles for FluidDatePicker (_fluid-date-picker.scss)
  • Stories for FluidDatePicker (FluidDatePicker.stories.js)
  • Tests for FluidDatePicker

Changed

  • Fixed some styles I noticed when adding in FluidDatePicker to the FluidForm story

Testing / Reviewing

Go to FluidDatePicker and check out the stories. For now, I've got all the different states in (they will be removed before merging). Ensure all playground controls are correct and stories render as expected.

@netlify
Copy link

netlify bot commented Oct 5, 2022

Deploy Preview for carbon-components-react ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 5675d84
🔍 Latest deploy log https://app.netlify.com/sites/carbon-components-react/deploys/636a8654d670990008972994
😎 Deploy Preview https://deploy-preview-12247--carbon-components-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Oct 5, 2022

Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit 5675d84
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/636a8654c8beef0008660180
😎 Deploy Preview https://deploy-preview-12247--carbon-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@tw15egan
Copy link
Member Author

tw15egan commented Oct 6, 2022

@aagonzales I have a lot of the base styles up, but curious how we want to handle invalid and warn styles for the range type... Currently, each DatePickerInput accepts an invalid and invalidText prop, and the error message is styled under each input respectively.
Screen Shot 2022-10-06 at 3 14 59 PM

I noticed in the spec we have an error state on the entire DatePicker itself.
Screen Shot 2022-10-06 at 3 15 58 PM

Would we want to allow both? i.e you could put invalid on the entire DatePicker, or only allow invalid on the DatePickerInput, which is how it is currently built. The entire FluidDatePicker would be styled as invalid, but the invalid message would just appear under the invalid input.

If we add it to the entire FluidDatePicker, should this also be added to the existing DatePicker?

@aagonzales
Copy link
Member

@tw15egan yeah we should probably allow just triggering invalid on one side ... I'm not yet sure how that will look visually. We might have to come back to that.

Also kind of on the same thread, I forget to spec out timePicker in fluid and will have a sam joint input problem.

@tw15egan
Copy link
Member Author

@aagonzales, so would the invalid outline span the entire input, but just keep invalid text to one input (as-is) for now? Since it's experimental we can come back and refine this interaction as well

@aagonzales
Copy link
Member

@tw15egan I'm going to talk about this in design crit tomorrow and hopefully get a final answer. The bottom+left is how in forms, we originally said we would treat inline fluid inputs with one invalid state.

image

@aagonzales
Copy link
Member

@tw15egan This is what we decided, is it doable?

image

@tw15egan
Copy link
Member Author

Yeah, we should be able to do that; we'll just need to add a prop of invalid to the DatePicker in addition to the DatePickerInput. Would this also be a valid use case outside of Fluid components? i.e., the entire DatePicker is invalid? If so, we may want to add that to the normal DatePicker as well

@tw15egan
Copy link
Member Author

I believe AVT tests are failing due to the duplicate ids on the stories showcasing all the different warning/error variants. Before merging, these will all be removed and set to a single story

Copy link
Member

@aagonzales aagonzales left a comment

Choose a reason for hiding this comment

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

The componet styles all look correct but there are some bugs in the Playground.

  • I'm getting some weird bugs when I switch between Range and Single.
    • Switch from single back to range and I can't get the end date calendar menu to open
    • Switch from range (with a range selected) to single, the range is still showing when I open the menu.
  • When invalid is turned on the width of the inputs grow. The error message should be wrapping and not effecting the width. (Same for warning)
    Oct-28-2022 13-49-45

@tw15egan
Copy link
Member Author

tw15egan commented Nov 1, 2022

@aagonzales That switcher is a bit temperamental, so I switched it to show two components. Also, I set a width on the container, so the component expands downwards now.

Copy link
Member

@tay1orjones tay1orjones left a comment

Choose a reason for hiding this comment

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

Looks great to me! 🎉 Just some minor suggestions for the playground story

tw15egan and others added 4 commits November 4, 2022 11:12
…stories.js

Co-authored-by: Taylor Jones <tay1orjones@users.noreply.github.com>
…stories.js

Co-authored-by: Taylor Jones <tay1orjones@users.noreply.github.com>
…stories.js

Co-authored-by: Taylor Jones <tay1orjones@users.noreply.github.com>
Copy link
Member

@aagonzales aagonzales left a comment

Choose a reason for hiding this comment

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

Looks good to go! 👍 ✅

@kodiakhq kodiakhq bot merged commit 5ee80eb into carbon-design-system:main Nov 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Fluid inputs: Add FluidDatePicker experimental implementation
4 participants