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.

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

[FEATURE] Création du bloc Statistiques et refacto (PIX-1200). #193

Merged
merged 5 commits into from
Oct 16, 2020

Conversation

alexandrecoin
Copy link
Contributor

@alexandrecoin alexandrecoin commented Oct 12, 2020

🦄 Problème

Il manque un bloc statistique contribuable par la team Contenu sur Prismic.

🤖 Solution

Ce nouveau bloc présentant les mêmes caractéristiques au niveau du template et du script que les blocs Feature et Process, nous avons opté pour une refacto de ces trois composants avec des appels à des fichiers SCSS externes et propres à chacun des blocs en fonction du type de ces derniers à créer sur Prismic.

  • Création d'un composant MultipleBlock (Prismic et code) reprenant le template et la logique des composants Process, Features et Stats
  • Création d'un dossier slices dans assets/scss afin de copier l'architecture du dossier de composants Vue (components/slices) avec ajout des styles des différents composants faisant appel à MultipleBlock
  • Update 14/10/2020 : Modification des couleurs utilisées dans les fichiers CSS afin de suivre le design system suite à la PR [TECH] Aligner les couleurs de pix-site #200

🌈 Remarques

Je me demande si MultipleBlock est un nom pertinent pour ce composant. 🧐
Je n'ai pas réussi à trouver quelque chose de succinct pour exprimer la réutilisation possible de ce composant par différents blocs dans Prismic.

✨ Review App

https://site-pr193.review.pix.fr/
https://pro-pr193.review.pix.fr/

@alexandrecoin alexandrecoin added 🚧 Development in progress team-evaluation PR relatives à l'expérience d'évaluation labels Oct 12, 2020
@alexandrecoin alexandrecoin self-assigned this Oct 12, 2020
components/SliceZone.vue Outdated Show resolved Hide resolved
components/slices/MultipleUse.vue Outdated Show resolved Hide resolved
@alexandrecoin alexandrecoin force-pushed the pix-1200-new-multiple-use-block branch 4 times, most recently from 17620b5 to 1a96585 Compare October 13, 2020 09:03
@pix-service
Copy link

I'm deploying this PR to these urls:

Please check it out!

@alexandrecoin alexandrecoin force-pushed the pix-1200-new-multiple-use-block branch 7 times, most recently from 2fe9df1 to 7d7438c Compare October 13, 2020 13:32
@alexandrecoin alexandrecoin marked this pull request as ready for review October 13, 2020 13:43
@alexandrecoin alexandrecoin force-pushed the pix-1200-new-multiple-use-block branch 2 times, most recently from 1980fce to 627f1db Compare October 13, 2020 14:32
@MelanieMEB
Copy link
Member

Question : est-ce que le passage de Feature/etc. à MultipleBlock coté Prismic demandera de modifier tous les composants existants ? Il faudrait du downtime ?

@alexandrecoin
Copy link
Contributor Author

Question : est-ce que le passage de Feature/etc. à MultipleBlock coté Prismic demandera de modifier tous les composants existants ? Il faudrait du downtime ?

Il faudra recréer chaque composant sur Prismic vu qu'il n'est pas possible de modifier le type d'un composant créé sur le document. (Il peuvent seulement être déplacés sur la page et/ou supprimés)

Du downtime ? A quel niveau ?

@alexandrecoin alexandrecoin force-pushed the pix-1200-new-multiple-use-block branch 2 times, most recently from d2a8b71 to cd93de5 Compare October 14, 2020 04:45
Copy link
Contributor

@sbedeau sbedeau left a comment

Choose a reason for hiding this comment

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

Techniquement ok avec quelques remarques.
La spécification des classes scss comme proposée permettrait de factoriser des lignes du css

@@ -53,6 +53,9 @@
<template v-if="slice.slice_type === 'process'">
<process-slice :slice="slice" />
</template>
<template v-if="slice.slice_type === 'multiple_block'">
<multiple-block-slice :slice="slice" />
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (non-blocking):

Suggested change
<multiple-block-slice :slice="slice" />
<key-elements-slice :slice="slice" />

components/slices/MultipleBlock.vue Outdated Show resolved Hide resolved
components/slices/MultipleBlock.vue Outdated Show resolved Hide resolved
components/slices/MultipleBlock.vue Outdated Show resolved Hide resolved
components/slices/MultipleBlock.vue Outdated Show resolved Hide resolved
components/slices/MultipleBlock.vue Outdated Show resolved Hide resolved
components/slices/MultipleBlock.vue Outdated Show resolved Hide resolved
Copy link
Contributor

@sbedeau sbedeau left a comment

Choose a reason for hiding this comment

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

Ça me semble ok fonctionnellement, bon refacto !

praise: Super PR, merci !

Copy link
Member

@MelanieMEB MelanieMEB left a comment

Choose a reason for hiding this comment

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

OK pour moi (et plutot OK avec les commentaires de steph)

@sbedeau
Copy link
Contributor

sbedeau commented Oct 15, 2020

thought (non-blocking): C'est peut-être une bonne occasion pour mettre des tests ?

@alexandrecoin alexandrecoin force-pushed the pix-1200-new-multiple-use-block branch from 65e1aa8 to 20c3396 Compare October 16, 2020 07:52
@alexandrecoin alexandrecoin force-pushed the pix-1200-new-multiple-use-block branch from 20c3396 to eb4d9bf Compare October 16, 2020 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Func Review OK team-evaluation PR relatives à l'expérience d'évaluation Tech Review OK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants