Scotto training part 13 calendar #18
Conversation
| @@ -0,0 +1,7 @@ | |||
| $menuHeight: 56vh | |||
There was a problem hiding this comment.
56vh cest balèze, ça veut dire que le menu fait 56% de la hauteur du viewport donc plus de la moitié du screen. Non là menuHeight doit vraiment être exprimé en pixels
There was a problem hiding this comment.
Je vois tout de suite que la variable n'est pas utilisée du coup. (parce qu'elle est obviously fausse)
There was a problem hiding this comment.
tout à fait mdr j'ai pas compris toute cette partie vh shit la :'( :'(
|
|
||
| $appContainerPadding: 2vh | ||
|
|
||
| $contentAvailableHeight: 100vh |
There was a problem hiding this comment.
Ca c'est faux. Justement le contenu n'a pas 100% du viewport disponible pour s'afficher. Quand je parle de content, je parle de ce qu'on va effectivement pouvoir sous la navbar, après avoir retiré la marge avec la navbar et le padding par défaut de l'app pour que l'app respire (et ne touche pas les bords).
Quand tu dis 100vh tu dis déjà "100% de la hauteur du viewport", donc tout le screen visible. Cette déclaration est uniquement destinée à être mise sur .App pour dire "mon app n'a jamais le droit de dépasser 100% du viewport". C'est cette déclaration, couplée à celle ci :
.app-container
margin: 10px
margin-top: 20px
height: calc(100% - (52px + 20 + 10)px)qui va nous permettre ensuite de faire des sous parties d'app scrollable indépendamment. Est-ce que tu comprends le délire ? Par exemple dans la partie notes pour avoir chaque liste scrollable indépendamment des autres contenus.
There was a problem hiding this comment.
In fine on aurait plutôt :
// In variables.sass
$contentMargins: 10px
$contentMarginTop: 20px
$contentAvailableHeight: calc(100vh - $containerMargins - $containerMarginTop)
...
// Dans index.sass
.app
height: 100vh
// On pourrait mettre ça dans une variable $sreenHeight si on voulait rester dans cette logique sémantique des variables
// et l'utiliser dans la définition de $contentAvailableHeight
.app-container
margin: $contentMargins
margin-top: $contentMarginTop
height: $contentAvailableHeight
...
// Dans calendar.sass
.calendar
height: 100%
// C'est tout, car du coup le calendrier va avoir une hauteur à 100% celle de son parent,
// et donc aura sa height à $contentAvailableHeight, c'est ce qu'on veut.
// Donc ton calendrier prendra visuellement tout l'espace "autorisé" par ton layout. Capish ?| height: 100% | ||
| border: 1px solid $grey | ||
|
|
||
| .header |
There was a problem hiding this comment.
Ici tu l'appelle header, plus haut tu l'appelles topbar, il faut que tu te décides dans le naming, car dans ton code là topbar et header référencent la même chose mais donne l'impression d'être différents.
| .header | ||
| color: $black | ||
| border-bottom: 0.5px solid $grey | ||
| height: $topbarHeight |
There was a problem hiding this comment.
Je suis pas sûr que ça soit tout à fait correct, tu as p-e plutôt envie d'avoir une hauteur de 50 avec 10px de marge entre les deux ?
| </div> | ||
| {weeks.map((week, weekIndex) => { | ||
| return ( | ||
| <div className="week" key={weekIndex}> |
There was a problem hiding this comment.
You know what ? Le plus logique serait qu'on utilise key={week[0].unix()} car comme ça chaque entrée dans la liste aura vraiment une représentation unique, si la même semaine se retrouve entre un mois et un autre, react pourra la réafficher tout de suite sans recalculer.
| {weeks.map((week, weekIndex) => { | ||
| return ( | ||
| <div className="week" key={weekIndex}> | ||
| {week.map((day, dayIndex) => { |
There was a problem hiding this comment.
de la même manière utiliser day.unix()
Regarde https://momentjs.com/docs/#/displaying/unix-timestamp/ pour comprendre ce que ça fait
| const firstSunday = moment().endOf('month').endOf('isoWeek'); | ||
| console.log(firstSunday) | ||
|
|
||
| const weeksDataStructure = [ |
There was a problem hiding this comment.
Il me semble qu'on avait trouvé mieux et que ça faisait partie de ton exercice à refaire et rerefaire, mais je suis plus sûr.
On avait pas trouvé un truc genre
const DISPLAYED_WEEKS_COUNT = 5;
const NUMBER_OF_DAY_PER_WEEK = 7;
...
const weeksDataStructure = new Array(DISPLAYED_WEEKS_COUNT).map(() => new Array(NUMBER_OF_DAY_PER_WEEK));
Si en le réécrivant je me rappelle bien de tout ça.... A refaire donc ! :)
|
|
||
| const weeks = weeksDataStructure.map((week, weekIndex) => { | ||
| return week.map((day, dayIndex) => { | ||
| return firstMonday.clone().add(weekIndex*7 + dayIndex, 'days') |
There was a problem hiding this comment.
ça ça me plait. Tu pourrais te débarasser des returns en retirant les { et }, pour être encore plus compact.
| import Day from "./Day"; | ||
|
|
||
| export default function Calendar () { | ||
| const [displayedMonth, setDisplayedMonth] = useState(moment().format('MMMM')); |
There was a problem hiding this comment.
Ca m'étonne, j'ai l'impression que tu as changé ta manière de définir le month. Je vais checker les anciens commits
There was a problem hiding this comment.
Ah non pardon.
Par contre moment() va te retourner la date actuelle. Toi tu veux plutôt avoir le premier jour du mois je dirais ? Genre moment().startOf('month') ? Puis quand tu fais format('MMMM') ca va te générer une string c'est pas ce que tu veux. Toutes tes data doivent rester sous forme de date. Tu utilises format seulement quand tu veux afficher quelque chose à l'utilisateur (c'est pas une règle absolue mais ça doit couvrir 99% des cas).
There was a problem hiding this comment.
Donc de ce que je vois, ton truc "marche" dans la mesure où cliquer sur les boutons change bien le mois affiché.
Mais ton truc ne marche pas vraiment parce que tu n'utilise pas le displayed month pour générer les jours affichés, donc ton calendrier n'est en fait pas mis à jour.
Est-ce que tu comprends ce qu'il faut que tu fasses pour corriger le problème ?
No description provided.