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.

Sign up for GitHub

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

Jump to bottom

Suppression de redux-form #671

Merged
merged 7 commits into from Sep 23, 2019
Merged

Suppression de redux-form #671

merged 7 commits into from Sep 23, 2019

Conversation

mquandalle
Copy link
Contributor

@mquandalle mquandalle commented Sep 12, 2019 •
edited

Fixes #632

La suppression de cette dépendance simplifie grandement le code, qui est plus concis (alors que l'on supprime une dépendance !) et plus clair (moins d'indirections). La PR est a parité de fonctionnalités. Cette refacto doit permettre d'enchaîner sur les modifications mentionnées dans #632.

@mquandalle mquandalle force-pushed the remove-redux-form branch 3 times, most recently from fd21cfe to ee92b3b Compare September 13, 2019 10:48
@mquandalle mquandalle marked this pull request as ready for review September 13, 2019 10:48
@mquandalle mquandalle force-pushed the remove-redux-form branch 5 times, most recently from cfd4fd0 to e9eddce Compare September 13, 2019 13:06
johangirod
johangirod reviewed Sep 16, 2019
View reviewed changes
Copy link
Contributor

@johangirod johangirod left a comment

Choose a reason for hiding this comment

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

👏 👏 👏 Une refacto vraiment top ! L'usage des hooks simplifie énormément la lecture et la logique. Et avoir la situation dans le state permet enfin de déplacer une la logique métier des composants vers les reducer 👍

source/components/PeriodSwitch.js Outdated Show resolved Hide resolved
source/components/TargetSelection.js Outdated Show resolved Hide resolved
source/components/conversation/Input.js Show resolved Hide resolved
source/components/conversation/Input.js Outdated Show resolved Hide resolved
@mquandalle
Gestion de l'état "situation" avec nos propres actions
7a5beb9 
En vue de la suppression de Redux-form, ce commit crée deux nouvelles
actions : UPDATE_SITUATION et UPDATE_PERIOD qui permettent de gérer le
state de la situation, en retrouvant le même résulat qu'avec l'ancienne
implémentation au niveau du `formattedSituationSelector`
@mquandalle mquandalle force-pushed the remove-redux-form branch 2 times, most recently from 98563f2 to 17810ac Compare September 17, 2019 10:49
@mquandalle
🔥 Suppression de redux-form
3fbd94b 
Supprime aussi redux-batched-action. Le code résultant est plus concis
(alors que l'on supprime une dépendance !), et plus clair car il y a moins
d'indirections pour se conformer aux API de redux-form.
@mquandalle
Ajout du lint pour les hooks React
dbbb67e 
Comme recommandé dans la documentation des hooks React, ajout des deux
linters suivants : react-hooks/rules-of-hooks et react-hooks/exhaustive-deps

Mise à jour des composants, en particulier les useEffect pour y spécifier
toutes les dépendances.
@mquandalle
Input permanent pour les objectifs du simulateur
8c9a401 
Nous alternions avant entre un <span /> et un <input /> selon le contexte

Fixes #558
@mquandalle
Copy link
Contributor Author

J'ai ajouté un commit qui laisse toujours l'input affiché pour les objectifs du simulateurs — plutôt que d'alterner entre input et span comme actuellement.

@mquandalle
Copy link
Contributor Author

Remarque : le dernier commit que j'ai poussé hier soit (pour mettre rules dans le state) est buggé. Le déploiement est toujours dispo sur le commit précédent.

@johangirod
Copy link
Contributor

Le deploy du dernier commit qui marche :
https://5d80bbbfe4532f0008308390--syso.netlify.com/s%C3%A9curit%C3%A9-sociale/ind%C3%A9pendant

Je te laisse regarder les bugs avant de reviewer :)

@johangirod
Copy link
Contributor

Petit bug sur la version anglaise :

chrome-capture (1)

@mquandalle
Ajout des règles dans le state Redux
0ddc2d0 
Déplace la logique de changement de période d'un component vers un reducer
@mquandalle
Copy link
Contributor Author

mquandalle commented Sep 18, 2019 •
edited

J'ai réparé le commit qui déplace les règles dans le state redux.

@johangirod J'ai pas trop compris le bug : c'est le positionnement du curseur qui saute à la fin (ce que je n'arrive pas à reproduire), ou le fait qu'il y ait des animations de changement de prix en trop ?

@mquandalle
🐛 Correction formatage de l'AnimatedValue
7a965e6 
Suppression de l'AnimatedValue pour l'objectif courant
Simplification du code de TargetSelection
Corrections CSS
@mquandalle mquandalle force-pushed the remove-redux-form branch 2 times, most recently from f224373 to da077aa Compare September 23, 2019 09:39
@mquandalle
Copy link
Contributor Author

Dans la foulée je voudrais centraliser les différentes fonctions de formatage et normalisation des valeurs qui sont éparpillées dans le code, mais je propose de merger cette PR indépendamment.

@mquandalle mquandalle merged commit f3e79f4 into master Sep 23, 2019
@mquandalle mquandalle deleted the remove-redux-form branch September 25, 2019 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@laem laem laem left review comments

@johangirod johangirod johangirod approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Suppression de Redux-form
3 participants
@mquandalle @johangirod @laem