Skip to content

Feature/purchases#5

Open
joaquinVidal1 wants to merge 11 commits into
mainfrom
feature/purchases
Open

Feature/purchases#5
joaquinVidal1 wants to merge 11 commits into
mainfrom
feature/purchases

Conversation

@joaquinVidal1

Copy link
Copy Markdown
Owner

No description provided.

Comment thread src/application/App.tsx Outdated
Comment thread src/application/App.tsx Outdated
const PurchasesNavigator = createNativeStackNavigator<PurchasesParamList>();
const AppNavigator = createDrawerNavigator();

const StoreFlow = () => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Podrias llamarlo StoreNavigator o incluso StoreScreen que es como lo llaman en la documentación. Mas que nada para que a primera vista se entienda que es lo que estas definiendo

Una buena practica es tener estos Navigators en archivos separados, en el caso de que tengas varios mas te hace mucho ruido definirlos todos en el mismo archivo. Lo ideal es que App.tsx te quede lo mas limpio posible

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

A que te referis?
Al AppNavigator? Le puse App para diferenciarlo de la store que sería solo la parte de comprar.
Al StoreFlow? Pensé que se nombraban así, en el curso en la app de tracks hacen eso. Avísame y lo cambio.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Perdón, no fui claro. Me referia al StoreFlow si
Olvidate de los nombres que te dije, porque mirando la doc otra vez vi que los nombra como quiere

Lo que si estaría bueno es eso de dividir ese componente para que no quede tan pesado, de la forma que te comenté arriba

Comment thread src/application/App.tsx Outdated
Comment thread src/features/cart/queries.ts

return (
<SafeAreaView style={{flex: 1}}>
<SafeAreaView edges={['right', 'bottom', 'left']} style={{flex: 1}}>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ese left lo pones por algo en particular?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

sea puse todos menos el top, porque sino queda un margen que se pone automáticamente

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Esta pantalla mantiene el header de react navigation no?
Por eso es que no usas el top ?

import {colors} from '../../shared/colors';
import {Purchase} from '../types/Purchase';

export const PurchaseListItem = ({purchase}: {purchase: Purchase}) => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Te dejo por acá la forma de definir componentes y pasarle Props de la documentación de RN https://reactnative.dev/docs/typescript

Básicamente los define como Functional Components de React y define el tipo de las props que le pasa el componente. Esta bueno seguir estos ejemplos de las doc porque seguramente lo vas a ver así en muchos proyectos

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

uh no la tenía
Muchas gracias!

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

lo puse en CartItem que tiene 3 parámetros

Comment thread src/features/shared/components/ProductsSeparator.tsx Outdated
Comment thread src/features/purchases/components/PurchaseListItem.tsx
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