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] Afficher une icône de progression pour les 2 dernières participations d'un prescrit à une campagne d'évaluation sur Pix Orga (PIX-14808) #10408

Conversation

ThomasBazin
Copy link
Contributor

@ThomasBazin ThomasBazin commented Oct 24, 2024

🦄 Problème

Sur Pix Orga : dans le tableau des résultats des participants à une campagne d'évaluation, seul le résultat de la dernière campagne s'affiche. Dans le cas où un prescrit a envoyé plusieurs résultats à une campagne, nous voulons connaître sa progression entre ses deux dernières participations partagées.

🤖 Proposition

Ajouter une colonne "Évolution" ainsi qu'un tooltip dans le tableau.
Récupérer la valeur "evolution" (cf PR #10343) renvoyée par l'API et afficher une icône correspondante dans la cellule du tableau.

🌈 Remarques

À la demande du design, nous avons ajouté une couleur aux icônes de progression (vert, rouge ou orange) ainsi qu'un tooltip explicatif au survol de l'icône.
En cas d'évolution nulle (1 seule participation du prescrit donc pas de point de comparaison), la cellule est vide.
La colonne ne s'affiche que pour une campagne à envois multiples.
Cette PR concerne uniquement les campagnes de type évaluation. Les collectes de profils feront l'objet d'une PR séparée.

💯 Pour tester

Sur Pix Orga:

  1. Sélectionner l'orga PRO Classic, la campagne PROASSMUL, et aller sur l'onglet résultats.
  2. Constater que les prescrits n'ayant qu'une participation ont une case vide pour l'évolution, alors que ceux ayant plusieurs participations ont une icône de progression de la bonne couleur
  3. Vérifier les tooltips (header et icône)
  4. Aller sur la campagne PROASSIMP (pas d'envois multiples) et constater que la colonne Evolution ne s'affiche pas.

@pix-bot-github
Copy link

Une fois les applications déployées, elles seront accessibles via les liens suivants :

Les variables d'environnement seront accessibles via les liens suivants :

@ThomasBazin ThomasBazin force-pushed the pix-14808-display-evolution-assessment-campaign-results branch from dc7f5a0 to 95d4a67 Compare October 24, 2024 17:39
@ThomasBazin ThomasBazin force-pushed the pix-14808-display-evolution-assessment-campaign-results branch 4 times, most recently from 1a47fbe to 18d8f60 Compare October 25, 2024 14:54
@ThomasBazin ThomasBazin marked this pull request as ready for review October 25, 2024 15:04
@ThomasBazin ThomasBazin requested a review from a team October 25, 2024 15:04
@ThomasBazin ThomasBazin force-pushed the pix-14808-display-evolution-assessment-campaign-results branch from 5a1b525 to 51aec1c Compare October 28, 2024 09:41
@xav-car xav-car added Func Review OK PO validated functionally the PR and removed 👀 Func Review Needed labels Oct 28, 2024
Comment on lines 23 to 26
<PixIcon
@name={{(getIconName @evolution)}}
aria-label={{t (getIconLabel @evolution)}}
/>
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion ajouter le if else içi pour gérer l'affichage de l'icon ou non

Copy link
Contributor

Choose a reason for hiding this comment

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

Ajouter un fichier de test qui permettra d'alléger les tests Assessement / Target Profile

Comment on lines 242 to 262
assert.ok(screen.getByRole('columnheader', { name: t('pages.campaign-results.table.column.ariaEvolution') }));
assert.ok(screen.getByText(t('pages.campaign-results.table.column.evolution')));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
assert.ok(screen.getByRole('columnheader', { name: t('pages.campaign-results.table.column.ariaEvolution') }));
assert.ok(screen.getByText(t('pages.campaign-results.table.column.evolution')));
const evolutionHeader = screen.getByRole('columnheader', { name: t('pages.campaign-results.table.column.ariaEvolution') });
assert.ok(evolutionHeader);
assert.strictEqual(evolutionHeader.innerText, t('pages.campaign-results.table.column.evolution'));

@@ -37,6 +39,146 @@ module('Integration | Component | Campaign::Results::AssessmentRow', function (h
// then
assert.ok(screen.getByText('10'));
});

module('evolution icons', function (hooks) {
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion ne tester qu'un cas puisque ce sera tester dans un test du composant à part

width: fit-content;
}

&--green{
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion

&__increase, &__decrease, &__stable

const evolutionCell = screen.getByRole('cell', { name: t('pages.campaign-results.table.evolution.increase') });
assert.ok(evolutionCell);
const evolutionCellContent = within(evolutionCell);
assert.dom(evolutionCellContent.getByRole('img')).hasClass('evolution-icon--green');
Copy link
Contributor

Choose a reason for hiding this comment

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

pour moi ça sert à R . on test une classe qui n'a aucune validité de l'affichage de la bonne couleur .

<TableHeader aria-label={{t "pages.campaign-results.table.column.ariaEvolution"}}>
<div class="assessment-list__evolution-header">
{{t "pages.campaign-results.table.column.evolution"}}
<ParticipationEvolutionTooltip />
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion/amélioration : Ce genre de composant est utilisé plusieurs fois une tooltip avec une icon .

Pour mutualiser cet usage on pourrait créer un composant ui "générique" avec par defaut l'icone . et lui passer en paramètre ce dont il a besoin pour fonctionner dans son context .

Copy link
Contributor

@xav-car xav-car Oct 29, 2024

Choose a reason for hiding this comment

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

Pensez à générer un id unique dans le composant. ( pas besoin de lui passer un id spécifique. voir uniqueId dans list.gjs )

@ThomasBazin ThomasBazin force-pushed the pix-14808-display-evolution-assessment-campaign-results branch 8 times, most recently from 7146fd7 to 79710e9 Compare October 30, 2024 08:04
@@ -0,0 +1,21 @@
import PixIcon from '@1024pix/pix-ui/components/pix-icon';
Copy link
Contributor

Choose a reason for hiding this comment

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

à supprimer, il n'est plus nécessaire

@@ -44,6 +46,20 @@ import CampaignAssessmentRow from '../results/assessment-row';
{{/if}}
<TableHeader>{{t "pages.campaign-results.table.column.results.label"}}</TableHeader>
{{#if @campaign.multipleSendings}}
<TableHeader aria-label={{t "pages.campaign-results.table.column.ariaEvolution"}}>
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion supprimer ariaEvolution, cette trad contient la même chose que la tooltip, cela fait doublon

Copy link
Contributor

@xav-car xav-car left a comment

Choose a reason for hiding this comment

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

tech func ok -🐶 🐕 🦖 🦦

@ThomasBazin ThomasBazin force-pushed the pix-14808-display-evolution-assessment-campaign-results branch 2 times, most recently from d47b106 to 8132514 Compare October 30, 2024 14:21
@pix-service-auto-merge pix-service-auto-merge force-pushed the pix-14808-display-evolution-assessment-campaign-results branch from 8132514 to 166be5e Compare October 30, 2024 14:22
@pix-service-auto-merge pix-service-auto-merge merged commit 1641a14 into dev Oct 30, 2024
8 checks passed
@pix-service-auto-merge pix-service-auto-merge deleted the pix-14808-display-evolution-assessment-campaign-results branch October 30, 2024 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants