Skip to content
This repository has been archived by the owner on Feb 28, 2023. It is now read-only.

MODUL-1106: Modifier l'affichage de la vue mois du datepicker #1057

Merged
merged 20 commits into from
Jul 10, 2019

Conversation

raphpare
Copy link
Member

@raphpare raphpare commented Jul 4, 2019

@ulaval/modul-components

PR Checklist

@raphpare raphpare requested a review from chuckmah as a code owner July 4, 2019 15:46
@raphpare raphpare requested review from Mboulianne, jfdion and Atiomi July 4, 2019 15:47
@jipigi jipigi self-requested a review July 4, 2019 19:05
Copy link
Contributor

@jipigi jipigi left a comment

Choose a reason for hiding this comment

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

Super, l'animation fait une belle job aussi !

Quand on voudra faire sélectionner une année, on ajoutera une props pour permettre d'afficher la vue mois / année à l'ouverture.

@raphpare
Copy link
Member Author

raphpare commented Jul 4, 2019

Quand on voudra faire sélectionner une année, on ajoutera une props pour permettre d'afficher la vue mois / année à l'ouverture.

Cette fonctionnalité fonctionne déjà en utilisant la prop initial-view:MBaseCalendarView

@jipigi
Copy link
Contributor

jipigi commented Jul 5, 2019

Super, j'avais pas vu que tu l'avais déjà codé 👍

Pour le initial-view=YEARS-MONTHS, ce serait bien de faire scrroller la liste d'années pour afficher l'année sélectionnée par défaut par le composant à l'ouverture et éviter de tomber sur la première année de l'intervalle. On va se servir de ça surtout pour choisir une date de naissance, donc le range sera genre de 1900 à 2000. Qu'est-ce que tu en penses ?

Un détail aussi, mais je mettre le rond gris sur le mois courant (comme le jour d'aujourd'hui) car même si le mois est sélectionné par défaut je trouve que ce serait plus logique de dit, voici le mois courant, mais tu ne l'as pas encore sélectionné. C'est un détail ;)

@raphpare raphpare added this to the 1.0.0-beta.111 milestone Jul 5, 2019
@chuckmah chuckmah added enhancement breaking change UI A breaking change that will impact visual labels Jul 8, 2019
Copy link
Contributor

@chuckmah chuckmah left a comment

Choose a reason for hiding this comment

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

Voir commentaire il faudra au minimun

  1. utiliser les InternalCleaveOptions
  2. ajouter immediate: true

src/components/datepicker/datepicker.ts Outdated Show resolved Hide resolved
src/components/datepicker/datepicker.ts Outdated Show resolved Hide resolved

const parts: string[] = DATE_FORMAT_REGEX.exec(value) as string[];
if (!parts || parts.length < 4) {
if (!DATE_FORMAT_REGEX.test(value)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

On devrait peut être utiliser Date.parse() ici au lieu de faire tout la poutine a bras avec un regex qui introduit une certaine complexité et possiblement des bug (car généralement une regex = 4 bugs :neckbeard: )

Par example, dans date-format on fait :
!isNaN(Date.parse(control.value)

Copy link
Member Author

Choose a reason for hiding this comment

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

Lorsque je mets isNaN(Date.parse(control.value) plusieurs tests unitaires plantent.

ModulDate › initialization › from a string › of a supported format › then a date with format '18/1/1' is a date comparable with date '2018-01-01'

ModulDate › initialization › from a string › of a supported format › then a date with format '18/1/18' is a date comparable with date '2018-01-18'

A single date state › when calling event (from abstract class) › month events › month-select › when selecting a month after the maximum date, the current month will be set to the maximum

A range date state › when calling event (from abstract class) › month events › month-select › when selecting a month after the maximum date, the current month will be set to the maximum

Copy link
Contributor

@Mboulianne Mboulianne Jul 10, 2019

Choose a reason for hiding this comment

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

et si tu fais isNaN(new Date(control.value))?

image

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, je suis d'accord que ça sort du scope de cette PR mais on peut garder ça en tête si modul-date nous cause des problèmes

src/components/datepicker/datepicker.ts Outdated Show resolved Hide resolved
src/components/datepicker/datepicker.ts Outdated Show resolved Hide resolved
@chuckmah chuckmah merged commit 00bf327 into develop Jul 10, 2019
@chuckmah chuckmah deleted the feature/MODUL-1106_datepicker-months-view branch July 10, 2019 18:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
breaking change UI A breaking change that will impact visual enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants