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

Input component 86937xxay #2

Closed
wants to merge 2 commits into from
Closed

Input component 86937xxay #2

wants to merge 2 commits into from

Conversation

Ereffe
Copy link
Member

@Ereffe Ereffe commented Jan 12, 2024

Screenshot_2024-01-13-00-22-15-561_host.exp.exponent-edit.jpg

import React, { useState } from 'react';
import { StyleSheet, Text, TextInput, View } from 'react-native';

interface CustomInputProps {
Copy link
Contributor

Choose a reason for hiding this comment

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

zamiast interface stosujmy type

onSubmit: (text: string) => void;
}

const CustomInput: React.FC<CustomInputProps> = ({
Copy link
Contributor

Choose a reason for hiding this comment

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

ogólnie są dwie opcje typowania komponentu. W zasadzie obie są okej pod warunkiem, że stosuje je każdy w aplikacji spójnie.
W tym przypadku ja proponuję taki schemat, bo po prostu sam taki używam na daily basis w pracy i się przyzwyczaiłem XD

const CustomInput = ({ ...propsy }: CustomInputProps) => ...

import React, { useState } from 'react';
import { StyleSheet, Text, TextInput, View } from 'react-native';

interface CustomInputProps {
Copy link
Contributor

Choose a reason for hiding this comment

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

każdy wie, że ten typ jest napisany do propsów komponentu poniżej, więc nie ma potrzeby pisać CustomInputProps. Wystarczy Props :)

onSubmit: (text: string) => void;
}

const CustomInput: React.FC<CustomInputProps> = ({
Copy link
Contributor

Choose a reason for hiding this comment

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

Nazwa zamiast CustomInput to Input

Comment on lines +35 to +53
const styles = StyleSheet.create({
container: {
marginBottom: 10,
},
label: {
fontSize: 16,
marginBottom: 5,
fontFamily: '',
},
input: {
borderWidth: 1,
borderColor: '#c7c4d6',
borderRadius: 10,
width: 300,
height: 40,
padding: 10,
fontSize: 16,
},
});
Copy link
Contributor

Choose a reason for hiding this comment

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

w projekcie można już korzystać ze stylowania zgodnie z dokumentacją. Docelowo chcemy tworzyć wersję w motywie jasnym i ciemnym i takie też inputy potrzebujemy w tym zadaniu

Comment on lines +24 to +29
<TextInput
style={styles.input}
value={text}
secureTextEntry={secureTextEntry}
onChangeText={setText}
onSubmitEditing={handleSubmit}
Copy link
Contributor

Choose a reason for hiding this comment

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

brakuje propsa defaultValue.
Ogólnie wydaje mi się, że raczej tutaj nie będzie potrzeby użycia hooka. Sprawdź czy przypadkiem metoda onSubmitEditing nie zawiera eventu jako 1 argumentu, z którego możesz wyciągnąć wartość inputa.

import React, { useState } from 'react';
import { StyleSheet, Text, TextInput, View } from 'react-native';

interface CustomInputProps {
Copy link
Contributor

Choose a reason for hiding this comment

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

dobrą techniką tutaj byłoby przyjęcie przez propsów dokładnie takich samych typów jak komponent TextInput. W ten sposób mamy pewność, że inputowi można przekazać jego wszystkie propsy a wyglądałoby to tak:
type Props = React.ComponentsPropsWithRef<typeof TextInput> & { jakieś nasze propsy, których TextInput nie zawiera, takie jak pewnie label }

secureTextEntry,
onSubmit,
}) => {
const [text, setText] = useState('');
Copy link
Contributor

Choose a reason for hiding this comment

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

jeśli już byś chciał użyć useState, co w tym przypadku jest zbędne, to na przyszłość zauważ, że w takim przypadku jak tutaj mamy czasami do czynienia z inputami, które mają domyślnie wpisaną jakąś wartość. Wówczas useState powinien przyjąć tą domyślną wartość jako wartość początkową, a nie pustego stringa

@wiktorrudzki
Copy link
Contributor

oprócz labelki, input powinien też zawierać potencjalny error, którego na razie brakuje na figmie, ale poprosiłem o dodanie go

@Ereffe Ereffe changed the title Added input component 86937xxay Input component 86937xxay Mar 12, 2024
@Ereffe Ereffe closed this Mar 12, 2024
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.

2 participants