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

Enhancement field level changes #1

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

Kousika13
Copy link
Contributor

@Kousika13 Kousika13 commented Nov 19, 2018

  • Removed the wrapper components
  • Added validations for time picker
  • Fixed issues in converting moment object of ant design time picker value to JS date object
  • Fixed mapping issues in the schema
  • Added submit button
  • Moved the transformation function inside the component
  • Passed the rest of the properties to the antd component using filterDOMProps()
  • Made the transform date function reusable

client/main.js Outdated
schema={schema}
onChange={onChange}
spacing={3}
modelTransform={transformModel}

Choose a reason for hiding this comment

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

The component should be make the transform inside itself

Copy link
Contributor Author

@Kousika13 Kousika13 Nov 20, 2018

Choose a reason for hiding this comment

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

Hi @cesarve77 , I have moved the transform function into the component

},
});

schema.messages({

Choose a reason for hiding this comment

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

It is a different approach and works, and in fact should be in the code for double checking
But the task is about a component, thats why schema.js and app.js are given.

You can solve this in the component, disabling not allowed hours.

But is ok, are easy fixes which solve with better communication

Copy link
Contributor Author

@Kousika13 Kousika13 Nov 20, 2018

Choose a reason for hiding this comment

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

okay @cesarve77. Could you please let me know whether should I move the validations into the component ?

Choose a reason for hiding this comment

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

You can use the followings properties of TimePicker to disabled not allowed times

disabledHours | to specify the hours that cannot be selected
disabledMinutes | to specify the minutes that cannot be selected
disabledSeconds | to specify the seconds that cannot be selected

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cesarve77, I have disabled the end time based on the start time selected in the time picker.


const DateRange = ({onChange, value: { start, stop }}) => (
<RangePicker value={[start, stop]} onChange={dates => onChange({start:dates[0], stop: dates[1]})} />
);

Choose a reason for hiding this comment

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

Will be great add the rest of the properties

const DateRange = ({onChange, value: { start, stop },...props}) => ( <RangePicker value={[start, stop]} onChange={dates => onChange({start:dates[0], stop: dates[1]})} {... filterDOMProps(props)} /> );

Now we can pass properties to the antd component

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cesarve77 , I have passed the props to the antd component using filterDOMProps()

@Kousika13 Kousika13 force-pushed the enhancement-field-level-changes branch from 510fe92 to 475c904 Compare November 22, 2018 17:03
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

3 participants