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

Include filterDOMProps in fields #211

Merged
merged 6 commits into from
Mar 1, 2017
Merged

Include filterDOMProps in fields #211

merged 6 commits into from
Mar 1, 2017

Conversation

Gervwyk
Copy link
Contributor

@Gervwyk Gervwyk commented Feb 28, 2017

Solves #203

@radekmie radekmie added the Type: Feature New features and feature requests label Mar 1, 2017
Copy link
Contributor

@radekmie radekmie left a comment

Choose a reason for hiding this comment

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

Please, keep imports aligned and correctly spaced.

@Gervwyk
Copy link
Contributor Author

Gervwyk commented Mar 1, 2017

Oops. Sorry. Fixed.

@janowsiany
Copy link
Contributor

@Gervwyk I think it will be ok if we add {...filterDOMProps(props)} in wrapField.js also. It will solve #207. @radekmie What do you think about this?

@@ -8,7 +8,6 @@ const SubmitField = ({className, inputRef, value, ...props}, {uniforms: {error,
htmlType="submit"
ref={inputRef}
type="primary"
{...props}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why have you removed it? We shouldn't use filterDOMProps here as it's not wrapped with connectField, but it's still needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh. Sorry. That was just a typo of working on to many things at once.. My bad.

@Gervwyk
Copy link
Contributor Author

Gervwyk commented Mar 1, 2017

@janowsiany I was also considering #207 but still wanted to experiment with it. I'll do some tests later today then we can combine it in this PR. @radekmie what do you think of including filterDOMProps in wrapField?

@radekmie
Copy link
Contributor

radekmie commented Mar 1, 2017

We have to choose one way somewhere in the future (2.0.0 probably, as it will be a breaking change): where all unused props go. To the wrapper? To the input? To the inner wrapper (if any)? To nowhere (discard)? Right now, it's depending on the theme and field.

@radekmie radekmie merged commit f054e29 into vazco:master Mar 1, 2017
@Gervwyk
Copy link
Contributor Author

Gervwyk commented Mar 1, 2017

I struggle to see how it will not be dependant on the theme and field. However, I think its a case of either assigning props to the wrapper or to the field on schema level - which will mean a breaking change or we can say to include a object for wrapper props and all other props are assigned on field level.
Something like:

new SimpleSchema({
    size: {
        type: String,
        allowedValues: ['xs', 's', 'm', 'l', 'xl'],
        uniforms: {
         showSearch: true,
         filterOption: (input, option) => option.props.value.toLowerCase().indexOf(input.toLowerCase()) >= 0
         wrapperProps: { 
              labelCol: {span: 3, offset: 12},
              wrapperCol: {span: 3, offset: 12}
        }
    }
})

Where showSearch and filterOption end up as field props and labelCol and wrapperCol on the wrapper component. Think this might be less of a breaking change... Not sure how this fit in with other themes.

@radekmie
Copy link
Contributor

radekmie commented Mar 1, 2017

Yep, there's also #185. All in all - it shouldn't be discussed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature New features and feature requests
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants