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: Hide the DatePicker button when readOnly is true #1234

Merged
merged 2 commits into from
Oct 12, 2020

Conversation

skvale
Copy link
Contributor

@skvale skvale commented Oct 5, 2020

Description

A readOnly DatePicker field should not have a button

Screen Shot 2020-10-05 at 3 13 55 PM

Breaking Change: A readOnly DatePicker field does not have an InputGroup Button

IE11
Screen Shot 2020-10-06 at 8 46 22 AM

@skvale skvale self-assigned this Oct 5, 2020
@netlify
Copy link

netlify bot commented Oct 5, 2020

Deploy preview for fundamental-react ready!

Built with commit fd4af75

https://deploy-preview-1234--fundamental-react.netlify.app

@jbadan
Copy link
Contributor

jbadan commented Oct 5, 2020

I hate to make this change more complicated, but do we need to do this in fundamental styles too?

Edit: there is no readOnly example in fundamental-styles, so we're safe to proceed here.

@bcullman
Copy link
Contributor

bcullman commented Oct 5, 2020

I hate to make this change more complicated, but do we need to do this in fundamental styles too?

Edit: there is no readOnly example in fundamental-styles, so we're safe to proceed here.

I came here to ask about this too:

I see it is not documented there, but what is the Fiori3 stance on what readonly fields should look like?

@jbadan
Copy link
Contributor

jbadan commented Oct 5, 2020

@bcullman It's not defined anywhere in the spec - DatePicker documentation mostly centers around the Calendar in the popover with very little about the actual input field.

@skvale
Copy link
Contributor Author

skvale commented Oct 6, 2020

Are we OK merging this and picking that up as a conversation in a Thursday meeting?

@bcullman
Copy link
Contributor

bcullman commented Oct 6, 2020

@bcullman It's not defined anywhere in the spec - DatePicker documentation mostly centers around the Calendar in the popover with very little about the actual input field.

if it's not documented, why do we think the solution is to hide the add-ons? I get that this is A possible solution, but I am not certain it is the right one. We need someone from UX to weigh in/followup on this

@skvale
Copy link
Contributor Author

skvale commented Oct 7, 2020

We talked with Brandon and he thought it made sense to go with this change as it currently is.

@skvale skvale merged commit 80bb5d6 into master Oct 12, 2020
@skvale skvale deleted the feat/breaking-change-read-only-datepicker-no-button branch October 12, 2020 13:04
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.

None yet

4 participants