Task/date field #147
Task/date field #147
Conversation
There was a problem hiding this comment.
Etrange cette diff, j'ai mis 2mn à voir que tu avais ajouté "prettier-eslint"...
| color: textColor, | ||
| fontFamily: 'PublicSans-Bold', | ||
| fontSize: 20, | ||
| lineHeight: 48, |
There was a problem hiding this comment.
Je ne sais pas si c'est important, mais la lineHeight est à 23,5 sur la maquette
| textColor = theme.sw.colors.primary.main; | ||
| borderColor = theme.sw.colors.primary.main; | ||
| backgroundColor = | ||
| theme.sw.colors.primary.main + theme.sw.transparency[16]; |
There was a problem hiding this comment.
Bon bah visiblement on fait comme ça les transparences x)
| hasError?: boolean; | ||
| } | ||
|
|
||
| function GetColorsStyle(theme: Theme, state: State) { |
There was a problem hiding this comment.
| function GetColorsStyle(theme: Theme, state: State) { | |
| function getColorsStyle(theme: Theme, state: State) { | |
| width: 72, | ||
| color: textColor, | ||
| fontFamily: 'PublicSans-Bold', | ||
| fontSize: 20, |
There was a problem hiding this comment.
On a pas une font size dans le DS ?
| return 'filled'; | ||
| } else { | ||
| return 'filled-focused'; | ||
| } |
There was a problem hiding this comment.
Peut-on aplatir les conditions ici ?
if (hasError) {
return 'error';
}
if (!isFocused && value === '') {
return 'empty';
}
if (isFocused && value === '') {
return 'empty-focused';
}
if (!isFocused) {
return 'filled';
}
return 'filled-focused';
| return style; | ||
| } | ||
|
|
||
| export const DateField: React.FC<DateFieldProps> = ({ |
There was a problem hiding this comment.
il n'a rien avoir avec la Date non ? et la seul chose qui lui donne accès au nombre c'est le clavier.
pourquoi ne pas laisser l'appelant choisir le clavier et l'appeler inputField ?
There was a problem hiding this comment.
Taille limité à 2 caractères et n'acceptant les chaines de caractères que composés de nombres (pas de virgules, points, ni lettres).
JosselinTILLAY
force-pushed
the
task/date-field
branch
from
5e890b7 to
3709a05
Compare
|
@AdrienCasta : tests ajoutés |
| @@ -1,87 +1,88 @@ | |||
| { | |||
| "name": "@zerogachis/smartway-react-native-ui", | |||
There was a problem hiding this comment.
C'est vrai que c'est bizarre cette diff, ça vient de quoi ? On a appliqué les règles qui ne l'avaient pas été jusque là à cause du dysfonctionnement du linter ?
| @@ -0,0 +1,160 @@ | |||
| import React, { useState } from 'react'; | |||
There was a problem hiding this comment.
Ce serait mieux de nommer le dossier "dateSelector" plutôt que datePicker
There was a problem hiding this comment.
C'est clément qui as choisit le nom, c'est un nom commun pour les sélecteurs de dates. J'ai pas d'avis sur la question. Je vais renommer, il y a un vote de ta part, de celle d'Adrien et de Lauren dans son figma ^^
|
|
||
| const style = getStyle(theme, state); | ||
|
|
||
| const onFocus = (e: NativeSyntheticEvent<TextInputFocusEventData>) => { |
There was a problem hiding this comment.
Est-ce que cet événement est déclenché même si le composant est déjà focus ou pas ?
There was a problem hiding this comment.
Hmm, c'est à dire ? Comment il pourrait être déjà focus ? Si je clique 3 fois dessus à la suite par exemple ?
There was a problem hiding this comment.
Yes en effet c'était ça, est-ce que si je clique plusieurs fois dessus je re déclenche l'événement, même si je suis déjà sélectionné ?
There was a problem hiding this comment.
Je viens de checker, l'évènement est déclenché uniquement quand on passe de l'état "unfocused" à l'état "focused"
JosselinTILLAY
force-pushed
the
task/date-field
branch
from
cec7784 to
41d9e4a
Compare
Pour info, on a utilisé la terminologie "placeholder" et pas "prefilled" parce que le comportement prefilled se situe au niveau du dateSelector, pas du dateFilled. C'est ouvert à discussion, c'est juste un terme :p