feat: disabled attribute for form fields
#4
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This pull request introduces a 'disabled' attribute for form fields across the Hyperview framework, enhancing form control and user experience.
- Implemented 'disabled' attribute in date fields, picker fields, select components, switches, and text inputs
- Added new style modifiers for disabled and selected-disabled states in stylesheets
- Updated component logic to handle disabled states, preventing user interaction when disabled
- Deprecated 'editable' attribute in favor of 'disabled' for text fields
- Enhanced type definitions to support new disabled functionality across components
19 file(s) reviewed, 18 comment(s)
Edit PR Review Bot Settings
| <modifier disabled="true"> | ||
| <style borderBottomColor="#FAFAFA"/> | ||
| </modifier> |
There was a problem hiding this comment.
style: Consider using a more distinct visual style for disabled fields
| <modifier disabled="true"> | ||
| <style borderBottomColor="#FAFAFA"/> | ||
| </modifier> |
There was a problem hiding this comment.
style: Consider adding more styles for the disabled state, such as a different text color or opacity
| @@ -132,6 +153,31 @@ tags: forms | |||
| </option> | |||
| <view style="Select__Separator"/> | |||
| </select-multiple> | |||
| <text style="Description">Disabled</text> | |||
| <select-multiple name="choice2" style="Select"> | |||
There was a problem hiding this comment.
logic: Duplicate name attribute 'choice2' used for disabled select-multiple
| <switch style="switch" name="switch1" value="on"/> | ||
| </view> | ||
| <view style="FormGroup"> | ||
| <text style="label">Switch on, disabled</text> | ||
| <switch style="switch" name="switch2" value="on" disabled="true"/> |
There was a problem hiding this comment.
logic: The name 'switch2' is used for both the enabled and disabled 'on' switches. Consider using unique names for each switch
| <style id="switch-styled" color="#E1E1E1" backgroundColor="red"> | ||
| <modifier selected="true"> | ||
| <style backgroundColor="#550000" color="#ffaaaa"/> | ||
| </modifier> | ||
| </style> |
There was a problem hiding this comment.
style: The styled switch doesn't have disabled states defined. Consider adding disabled and selectedDisabled modifiers for consistency
| const selectedDisabledStyle = StyleSheet.flatten( | ||
| createStyleProp(this.props.element, this.props.stylesheets, { | ||
| disabled: true, | ||
| selected: true, | ||
| }), | ||
| ); |
There was a problem hiding this comment.
logic: selectedDisabledStyle is defined but not used. Consider applying this style when the switch is both selected and disabled.
| const editableAttr = props.element.getAttribute('editable'); | ||
| if (editableAttr) { | ||
| console.warn( | ||
| 'Deprecation notice: `editable` attribute is deprecated and will be removed in a future version. Use `disabled` attribute instead.', | ||
| ); | ||
| } | ||
| const editable = | ||
| editableAttr !== 'false' && | ||
| props.element.getAttribute('disabled') !== 'true'; |
There was a problem hiding this comment.
style: Consider using a separate function for attribute parsing and deprecation warnings to improve readability and maintainability.
| disabled: StyleSheet, | ||
| selectedDisabled: StyleSheet, |
There was a problem hiding this comment.
style: Consider adding comments to explain the purpose of these new style properties
| disabled?: ?boolean, | ||
| selectedDisabled?: ?boolean, |
There was a problem hiding this comment.
style: Add JSDoc comments to describe these new options
| disabled?: ?boolean, | ||
| selectedDisabled?: ?boolean, |
There was a problem hiding this comment.
style: Ensure consistent naming: consider using isDisabled instead of disabled for boolean props
Resolves Instawork#292