DatePicker: optional underlined prop, like for TextField #6144
Conversation
|
@betrue-final-final let me know what you think |
| value={date} | ||
| /> | ||
| )) | ||
| .add('Underlined', () => ( |
There was a problem hiding this comment.
can you add a few of the variants here? At least required
| * Whether or not the Textfield of the DatePicker is underlined. | ||
| * @default false | ||
| */ | ||
| underlined?: boolean; |
There was a problem hiding this comment.
my only thought here is if there is value in simply exposing the Textfield props. It does hide this mode a little bit, but avoids us deprecating it in the future if we decide to expose all the Textfield props.
There was a problem hiding this comment.
Hmm interesting thought. I'm not sure if we'd ever want to expose TextField props like mask, maskChar, prefix, suffix, errorMessage, etc., but maybe I'm just overlooking a use case for those. @betrue-final-final what do you think?
There was a problem hiding this comment.
nah, that's a good point. there are a TON of things in there that don't make sense to expose here. If anything, we'd want to share some lower level abstraction between datepicker and Textfield.
I'm good with this as is, and if we ever come to a situation where we need to change it, we'll do it then.
|
|
|
That's probably from the disabled DatePicker. I'll look into it |
|
looks like 1 change incoming. Focus color of the bottom line should be primary like Textfield
|
|
To me, it looks like it has the same behavior as the normal DatePicker when opened. Do we want different behavior?
|
|
well that's kinda annoying. The non underline versions are different. I agree that we should be consistent within component for now. Changing the datepicker to match Textfield is a bigger conversation. Happy to have the conversation, but don't want to hold up this PR to make that pretty significant change. |
|
Also, the black is also how it's done currently in production
|
|
Will ok for now. FYI it's out of sync with SharePoint. |
|
@lorejoh12 just a heads up that we're adding this variant to the datepicker. We noticed that you have your own version already, hopefully this alleviates the need for as many overrides. As for discrepancy between outlook at SharePoint, is there design guidance there? Is moving towards black underline where your design is headed, and SharePoint should consider following? |
Pull request checklist
$ npm run changeDescription of changes
This feature was pretty easy to add, but it'd be great if design could look over it, too! It is an optional prop, so you'd only see a difference when passing
underlinedinto DatePicker. Otherwise, it is rendered normally. I added a VR test as well and some screenshots below:Microsoft Reviewers: Open in CodeFlow