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

adding scenarios and adding screen #19

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

Conversation

BrunnaSilva
Copy link

No description provided.

@@ -1,5 +1,5 @@
export default {
useReactotron: __DEV__,
useFixtures: false,
useStorybook: false
useFixtures: true,

Choose a reason for hiding this comment

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

Quando a gente sobe algum código pra develop ou pra master, tentamos sempre subir com fixtures desativadas. Isso evita eventuais erros na hora de gerar versões de produção.


type Props = {
item : ToDoAdd,
onToDos(values: ToDoAdd):void

Choose a reason for hiding this comment

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

Para tipagem de funções, você pode definir assim:


type Props = {
    item: ToDoAdd,
    onToDos: (values: ToDoAdd) => void
}
  • Tente definir nomes com verbos para callbacks. Por exemplo, em vez de onToDo, definir onAddToDo ou onAddItem. Como você escolheu o termo item no prop acima, era legal padronizar.

onToDos(values: ToDoAdd):void
}

const AddToDO = ({ item, onToDos}: Props,ref) => {

Choose a reason for hiding this comment

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

De onde veio esse ref? 🤔


import styles from './ToDoScreen.style'
import { Images } from '../../../Themes'

import type { StackNavigationProp } from '@react-navigation/stack'

import moment from 'moment'
import moment from 'moment' //data-hora
import { TextInput } from 'react-native-gesture-handler'

Choose a reason for hiding this comment

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

Por que usar o TextInput do react-native-gesture-handler e não o do react-native? 🤔 A não ser que ele faça alguma coisa especial, é mais seguro usar os componentes 'nativos' ao react-native.

@@ -41,36 +50,49 @@ const ToDoScreen = ({ navigation }: Props) => {

// Consts
const filterList = ['All', 'Today', 'This week', 'This month']
console.tron.logImportant({sortedToDos,fetching,error})

Choose a reason for hiding this comment

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

Numa PR normal, comentários ou console.logs no código geralmente fazem a tarefa voltar, pra você retirar.

const ToDoScreen = ({ navigation }: Props) => {
// Redux Actions
const dispatch = useDispatch()
const getToDos = useCallback(() => dispatch(ToDosUIActions.request()))

// State
const [selectedFilterIndex, setFilterIndex] = useState(0)
const [selectedFilterIndex, setFilterIndex] = useState(0)
const [add, setAdd] = useState(false)

Choose a reason for hiding this comment

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

Com propriedades booleanas, é recomendado seguir o padrão de adicionar um is ou are no início da frase. Por exemplo:

const [isAdding, setIsAdding] = useState(false)
const [isOpen, setIsOpen] = useState(false) 

ou se você preferir (com tipagem do Typescipt):

const [isAdding, setAddingStatus] = useState<boolean>(false)
const [isOpen, setOpenStatus] = useState<boolean>(false) 

Copy link
Contributor

Choose a reason for hiding this comment

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

E seria interessante também deixar mais claro o que está sendo adicionado, aberto ou o que a data representa, por exemplo:

const [isAddingSomething, setAddingSomething] = useState<boolean>(false)
const [isSomethingOpen, setIsSomethingOpen] = useState<boolean>(false)
const [reminderDate, setReminderDate] = useState<Date>(new Date())

</View>
)

const FloatingButton = ({ onPress }) => (

Choose a reason for hiding this comment

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

Esses componentes dentro desse arquivo são bem pequenos, mas ainda assim, merecem seu próprio arquivo, seu próprio arquivo de estilos, seu próprio registro no Storybook, etc. Inclusive, são esses componentes pequenos que são super reutilizáveis e não podem estar perdidos dentro de um arquivo de tela.

</View>
)

const PriorityList = () =>{

Choose a reason for hiding this comment

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

Um erro bastante comum é guardar informações de estado de dados dentro do componente e não da tela. Por exemplo, a tela de "Novo Lembrete" precisa da prioridade, mas esse informação á perdida aqui dentro desse componente. Para isso, a gente geralmente usa o padrão controlled-component, onde a tela controla o componente. Ficaria algo assim:

const ToDoScreen = () => {
  const [selectedPriority, onSelectPriority] = useState('Selecionar')
  return (.... 
     <PriorityList 
        selectedPriority={selectedPriority} 
        onSelectPriority={onSelectPriority} 
     /> 
  ...)
}

const PriorityList = ({ selectedPriority, onSelectPriority }) => (...)

@@ -8,3 +8,9 @@ export type ToDoType = {
reminder: string,
priority: string
}

export type ToDoAdd = {

Choose a reason for hiding this comment

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

Um ToDo é um item, não uma ação, então não precisa do "Add" no final.

  1. Existe um ToDo que não tem título? Até consigo imaginar não ter lembrete ou prioridade, mas título é importante.
  2. Geralmente não colocamos o | null direto na entidade, pq aí, em todo local que ele é usado no app, ele poderá ser nulo, e isso não é o que queremos. Em lugares específicos, e gente pode adicionar o | null mas não na definição da entidade.

@@ -133,6 +133,8 @@ android {
targetSdkVersion rootProject.ext.targetSdkVersion
versionCode 1
versionName "1.0"
//adicional b

Choose a reason for hiding this comment

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

Por que precisou adicionar isso? 🤔

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.

3 participants