Skip to content

Code review Oleg Marin #3

Description

@olegMarin
  1. класть какой то экран с какими то активными компонентами в App.js - это плохая практика, я такое не поддерживаю.
    в корне мы размещаем анимацию загрузки приложения и обёртку навигатора. можно ещё запросить актуальность токена авторизации, чтобы в навигаторе сразу выбрать нужный стек экранов.

так же тут же сразу обратил внимание что используются двойные кавычки. в реакте это признак дурного тона. двойные кавычки мы используем только когда передаём в параметр строковое значение. во всех остальных случаях используем одинарные кавычки - это легче читается и вообще соответствует отраслевым нормам

  1. продолжим придирки к оформлению. очень сложно читать вот такие компоненты когда все параметру в строку. больше одного - двух пропсов в компоненьте, пожалуйста, каждый с новой строки. ну правда же красивее было бы.

<Text style={ styles.userContent } accessibilityLabel={translations["Private content"]} testID={translations["Private content"]}>{ translations["private1"] }</Text>

  1. экраны вообще сразу на вырост лучше создавать в папках где название компонента - это название папки, внутри index.js файл с этим компонентам и дальше если внутри простые компоненты, раскидываем их внутри папки, если компоненты будут содержать себе какие то локальные решения - так же создаём папку с названием локального компонента и внутри index с этим компонентом. иначе будет грусть и печаль как только приложение начнёт расти и его будет разрабатывать больше одного человека одновременно.

  2. мне не наравится пробел в ключе 'Private content' и 'Login button'. я бы сделал 'privateContent' и 'loginButton'

    <Text style={ styles.userContent } accessibilityLabel={translations["Private content"]} testID={translations["Private content"]}>{ translations["private1"] }</Text>

  3. объект со стилями лучше выделить в файлик рядом и импортировать в index где компонент. ну это естественно когда у нас компоненты по папочкам разложены

    const styles = StyleSheet.create({

  4. const [ isRemember, toggleIsRemember ] = useState(false)... в остальных исключительно бинарных значениях то же самое. это позволит видеть человеку, работающему с кодом, что мы просто передаём обратное значение в стейт

    const [ remember, setRemember ] = useState(false)

  5. я против любых стрелочных функций, даже самых элементарных, внутри JSX. давайте выносить всё в функции внутри компонента и просто класть эту константу в пропс. и код будет гладко работать и читать проще и все короче в профите.

например:
const handleToggleRemember = (e) => toggleIsRemember(!remember);
.....
<TouchableOpacity onPress={handleToggleRemember}>

<TouchableOpacity onPress={e => setRemember(!remember)}>

  1. это тоже придирки, но alignItems давайте писать после disolay flexDirection и justifyContent

  1. надо разделять вообще компоненты в ту папку которая есть и экраны в папку screens. даже в таком маленьком приложении получается бардак. в редаксе почему то порядок, а там где много всего - все свалено в одну кучу.
    https://github.com/autotests-cloud/react-native-web-ios-android-app/blob/master/src/components/LangComponent.js

в остальных компонентах в принципе у меня глаза на том же самом зацикливаются, может потом ещё что нить можно посмотреть.

Metadata

Metadata

Labels

No labels
No labels

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions