-
-
Notifications
You must be signed in to change notification settings - Fork 31.7k
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
[Form components] Set cursor:not-allowed
style when disabled
#4170
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -59,6 +59,7 @@ const getStyles = (props, context, state) => { | |
outline: 'none', | ||
backgroundColor: 'rgba(0,0,0,0)', | ||
color: props.disabled ? disabledTextColor : textColor, | ||
cursor: props.disabled ? 'not-allowed' : 'initial', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 'initial' is not supported by IE. This means that if I start with a disabled TextField, and later enable it, the cursor is still There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @TildeWill There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@TildeWill Could you develop on this point? We would definitely accept a PR for this. |
||
font: 'inherit', | ||
}, | ||
textarea: {}, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need a
not-allowed
cursor here?What would be the use case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From docs:
disableYearSelection
- Disables the year selection in the date picker.I thinks, this is the use-case.
Could you please explain, why this is not the use-case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing the
disableYearSelection
property doesn't change the styling of the element.Hence, I don't think that we should change the
cursor
.Why should we use the
not-allowed
? I think that we should us it toemphasize
the disabled state.So when should we use
not-allowed
? I would say as soon as we change the default style.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point, agree. But I think, in this particular case we should emphasize disabled case.
Property
disableYearSelection
can be dependent on some custom application component property.And this is case for the emphasizing - in come cases this field will be
true
, in somefalse
. And year field in UI will be interactive or not.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I agree as the default value of
disableYearSelection
isfalse
👍 .