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.

Sign up for GitHub

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

Various tweaks for compatibility with react-native-web #3

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

Various tweaks for compatibility with react-native-web #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
900dd6e
fix: add various fixes for compatability with the web
DaleWebb Feb 10, 2021
aafcf73
fix(label): return styles and calc to prev values for platforms other…
DaleWebb Feb 16, 2021
2df90fa
fix: missing import
DaleWebb Mar 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/components/affix/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import PropTypes from 'prop-types';
import React, { PureComponent } from 'react';
import { Animated, Text } from 'react-native';
import { Animated } from 'react-native';

import styles from './styles';

Expand All @@ -11,7 +11,7 @@ export default class Affix extends PureComponent {

static propTypes = {
numberOfLines: PropTypes.number,
style: Text.propTypes.style,
style: PropTypes.any,

color: PropTypes.string.isRequired,
fontSize: PropTypes.number.isRequired,
Expand Down
2 changes: 1 addition & 1 deletion src/components/counter/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export default class Counter extends PureComponent {
baseColor: PropTypes.string.isRequired,
errorColor: PropTypes.string.isRequired,

style: Text.propTypes.style,
style: PropTypes.any,
};

render() {
Expand Down
26 changes: 8 additions & 18 deletions src/components/field/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,10 @@ import PropTypes from 'prop-types';
import React, { PureComponent } from 'react';
import {
View,
Text,
TextInput,
Animated,
StyleSheet,
Platform,
ViewPropTypes,
} from 'react-native';

import Line from '../line';
Expand Down Expand Up @@ -66,8 +64,6 @@ export default class TextField extends PureComponent {
};

static propTypes = {
...TextInput.propTypes,

animationDuration: PropTypes.number,

fontSize: PropTypes.number,
Expand All @@ -84,9 +80,9 @@ export default class TextField extends PureComponent {

labelOffset: Label.propTypes.offset,

labelTextStyle: Text.propTypes.style,
titleTextStyle: Text.propTypes.style,
affixTextStyle: Text.propTypes.style,
labelTextStyle: PropTypes.any,
titleTextStyle: PropTypes.any,
affixTextStyle: PropTypes.any,

tintColor: PropTypes.string,
textColor: PropTypes.string,
Expand Down Expand Up @@ -117,8 +113,8 @@ export default class TextField extends PureComponent {
prefix: PropTypes.string,
suffix: PropTypes.string,

containerStyle: (ViewPropTypes || View.propTypes).style,
inputContainerStyle: (ViewPropTypes || View.propTypes).style,
containerStyle: PropTypes.any,
inputContainerStyle: PropTypes.any,
};

static inputContainerStyle = styles.inputContainer;
Expand Down Expand Up @@ -453,15 +449,9 @@ export default class TextField extends PureComponent {
inputProps() {
let store = {};

for (let key in TextInput.propTypes) {
if ('defaultValue' === key) {
continue;
}

if (key in this.props) {
store[key] = this.props[key];
}
}
Object.keys(this.props).forEach((key) => {
store[key] = this.props[key];
});

return store;
}
Comment on lines 449 to 457
Copy link
Owner

@TheLartians TheLartians Feb 11, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Why not just return this.props at this point? Also unsure if this could trigger warnings / unexpected behaviour if arbitrary props get forwarded to the TextInput component below.

How about adding a whitelist with the supported TextInput props and using it to filter the props?

Copy link
Author

@DaleWebb DaleWebb Feb 11, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, both good points!

Why not just return this.props at this point?

I opted to retain the loop to indicate where an escape clause for a prop could be easily added in the future, if the previously existing escape clause needed to be added again (e.g. "defaultValue" === key).

Also unsure if this could trigger warnings / unexpected behaviour if arbitrary props get forwarded to the TextInput component below.

True - an oversight on my part as I'm a TypeScript user and rely on TS to warn of invalid props.

How about adding a whitelist with the supported TextInput props and using it to filter the props?

I thought about this and I wasn't sure if the chance of an error in missing out a supported prop (either by mistake or because react-native adds one in the future) was worth the reward. Also, conscious of React Native not supporting prop types any more.

Happy for suggestions on what you think the best approach for this would be if you can think of an alternative to supporting a copied array of supported props.

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah, I'm a TypeScript user myself, so I'm also all in favour of removing the propTypes overhead. The issue however is that the outer component accepts more props than a standard TextInput. Therefore, TypeScript should also accept these additional parameters without emitting a warning either.

The original uses TextInput.propTypes as a whitelist to filter out the TextInput props out from the full TextField props, so that nothing unexpected ends up there. Perhaps a blacklist would be better suited in the future - we already know which props are meant for the outer TextField component (e.g. by checking the propTypes defined above), so removing them should leave us with the props meant for the TextInput.

Expand Down
4 changes: 2 additions & 2 deletions src/components/helper/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import PropTypes from 'prop-types';
import React, { PureComponent } from 'react';
import { Animated, Text } from 'react-native';
import { Animated } from 'react-native';

import styles from './styles';

Expand All @@ -11,7 +11,7 @@ export default class Helper extends PureComponent {

disabled: PropTypes.bool,

style: Text.propTypes.style,
style: PropTypes.any,

baseColor: PropTypes.string,
errorColor: PropTypes.string,
Expand Down
6 changes: 3 additions & 3 deletions src/components/label/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import PropTypes from 'prop-types';
import React, { PureComponent } from 'react';
import { Animated, Text } from 'react-native';
import { Platform, Animated } from 'react-native';

import styles from './styles';

Expand Down Expand Up @@ -43,7 +43,7 @@ export default class Label extends PureComponent {
y1: PropTypes.number,
}),

style: Text.propTypes.style,
style: PropTypes.any,
label: PropTypes.string,
};

Expand Down Expand Up @@ -104,7 +104,7 @@ export default class Label extends PureComponent {
}, {
translateX: labelAnimation.interpolate({
inputRange: [0, 1],
outputRange: [x0, x1],
outputRange: [x0, Platform.select({default: x1, web: x1 - fontSize})],
}),
}],
};
Expand Down
10 changes: 6 additions & 4 deletions src/components/label/styles.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import { StyleSheet } from 'react-native';
import { StyleSheet, Platform } from 'react-native';

export default StyleSheet.create({
container: {
...Platform.select({default: {
left: '-100%',
width: '200%',
paddingLeft: '50%'
}, web: {}}),
position: 'absolute',
top: 0,
left: '-100%',
width: '200%',
paddingLeft: '50%',
},

text: {
Expand Down