-
Notifications
You must be signed in to change notification settings - Fork 18
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
base: master
Are you sure you want to change the base?
Conversation
app/App/Config/DebugConfig.js
Outdated
@@ -1,5 +1,5 @@ | |||
export default { | |||
useReactotron: __DEV__, | |||
useFixtures: false, | |||
useStorybook: false | |||
useFixtures: true, |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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
, definironAddToDo
ouonAddItem
. Como você escolheu o termoitem
no prop acima, era legal padronizar.
onToDos(values: ToDoAdd):void | ||
} | ||
|
||
const AddToDO = ({ item, onToDos}: Props,ref) => { |
There was a problem hiding this comment.
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' |
There was a problem hiding this comment.
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}) |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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 }) => ( |
There was a problem hiding this comment.
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 = () =>{ |
There was a problem hiding this comment.
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 = { |
There was a problem hiding this comment.
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.
- Existe um ToDo que não tem título? Até consigo imaginar não ter lembrete ou prioridade, mas título é importante.
- 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.
app/android/app/build.gradle
Outdated
@@ -133,6 +133,8 @@ android { | |||
targetSdkVersion rootProject.ext.targetSdkVersion | |||
versionCode 1 | |||
versionName "1.0" | |||
//adicional b |
There was a problem hiding this comment.
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? 🤔
No description provided.