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

Add Material TextField #245

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

Add Material TextField #245

wants to merge 15 commits into from

Conversation

adrianicv
Copy link
Contributor

According to #78, we needed an implementation of material textfields. I've implemented and approach based on https://github.com/n4kz/react-native-material-textfield/ repository but I've changed the API in order to fit the react native-material-ui philosophy getting the styles from the theme and changing other minor aspects. This is a first version, maybe after some usage we will detect some bugs and upgrades, but I think, for now, this is functional enough.

@xotahal please check it, and if there aren't any problems merge it.

Thank you.

@codecov-io
Copy link

codecov-io commented Dec 12, 2017

Codecov Report

Merging #245 into master will decrease coverage by 1.98%.
The diff coverage is 69.38%.

@@            Coverage Diff             @@
##           master     #245      +/-   ##
==========================================
- Coverage   76.61%   74.62%   -1.99%     
==========================================
  Files          19       25       +6     
  Lines         774     1068     +294     
  Branches      113      190      +77     
==========================================
+ Hits          593      797     +204     
- Misses        150      221      +71     
- Partials       31       50      +19

@piranna
Copy link

piranna commented Dec 23, 2017

Tests are not passing...

@adrianicv
Copy link
Contributor Author

@piranna, tests are passing as you can see in the details of ci/circleci. Nevertheless, coverage checks are not passing due to a reduction in the global coverage after the possible merge.

I agree that we should improve testing coverage, but now all tests, including master's tests, are only covering all the possible combination of snapshots without changes of state or interaction with the components. So, I think that this PR can be merged even if it reduces the global coverage and include in the roadmap the improvement of the tests.

@canercandan
Copy link

@adrianicv @piranna any news on this PR?

@piranna
Copy link

piranna commented Feb 7, 2018

@adrianicv @piranna any news on this PR?

No, just waiting to get it merged.

@xotahal
Copy link
Owner

xotahal commented Feb 7, 2018

Sorry guys, I will try to look at this soon. I was moving to New Zealand. So I didn't have so much time ..

@charleswong28
Copy link

charleswong28 commented Jun 6, 2018

Experiencing this error when compiling production apk using this pull request branch using RN-0.55.4.

com.facebook.react.common.JavascriptException: undefined is not an object (evaluating 's.View.propTypes.style'), stack:

As suggested by this issue, I searched in node_modules and found View.propTypes.style in TextField.react.js.

I suggest to use ViewPropTypes in utils instead.

import { ViewPropTypes } from '../utils';

....
style: PropTypes.shape({
        inputContainer: ViewPropTypes.style,
        container: ViewPropTypes.style,
        labelText: Text.propTypes.style,
        titleText: Text.propTypes.style,
        affixText: Text.propTypes.style,
        input: TextInput.propTypes.style,
}),

I've forked another branch to try. Let me know what you guys think @adrianicv @xotahal. I can help create another pr.

@sraka1
Copy link

sraka1 commented Feb 19, 2019

@xotahal any updates on this?

@brno32
Copy link

brno32 commented Jul 19, 2019

I would find this useful. Are there plans to merge this?

@mixa9269
Copy link

Any updates on this?

@brno32
Copy link

brno32 commented May 15, 2020

I was learning React back when I found this issue and made the above comment. Now I found a job as a react dev. Any updates?

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.

9 participants