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 un simulateur dans Modulix (PIX-13092) #9488

Merged
merged 9 commits into from
Jul 22, 2024

Conversation

reibecca
Copy link
Contributor

@reibecca reibecca commented Jul 11, 2024

🦄 Problème

Aujourd'hui, le référentiel Modulix peut contenir des embed sans complétion requise. L'API pouvant désormais les consommer, nous souhaitons pouvoir les afficher côté client.

🤖 Proposition

Implémenter un composant ModulixEmbed.
Ce composant ne prend pas en compte les messages envoyés par l'iframe qu'il contient.
Le composant contient un bouton "commencer" pour lancer l'embed, ainsi qu'un bouton "réinitialiser" afin de remettre l'embed à zéro.

  • Ajouter un composant Embed.gjs
  • Ajouter ce nouveau composant dans la gestion des éléments (switch/case)
  • Afficher un embed dans un grain
  • Ajouter un overlay sur l'embed avant qu'il ne soit lancer (iframe - blur)
  • Ajouter un bouton Commencer + méthode pour commencer l'embed
  • Ajouter un bouton Réinitialiser + méthode pour réinitialiser l'embed
  • Rajouter la consigne côté API
  • Rajouter la consigne côté front
  • Utiliser un embed + récent

🌈 Remarques

Ajout de la consigne dans le modèle

Dans les tâches précédentes, nous avions oublié de prendre en compte le besoin de consignes dans les embed.
Il a donc fallu l'ajouter dans cette PR pour qu'elle soit fonctionnellement complète.
Étant donné que dans l'existant, les embed ne comportent pas toujours de consigne, nous l'avons considéré optionnel dans notre modèle métier.

Validation Joi

Dans la validation Joi, le htmlSchema était required par défaut. Mais étant donné que la consigne des embed est optionnelle, nous avons retiré le caractère requis du htmlSchema, et l'avons remonté spécifiquement dans la définition des schémas de QCU, QCM et QROCM.

💯 Pour tester

  1. Se rendre sur le didacticiel
  2. Passer tous les grains jusqu'à celui comportant le simulateur
  3. Commencer le simulateur en cliquant sur le bouton "Commencer"
  4. Cliquer sur l'icône "caméra" au sein du simulateur
  5. Cliquer sur le bouton "Réinitialiser"
  6. Constater que le simulateur a retrouvé son état initial
  7. Répondre à la question qui accompagne le simulateur
  8. Constater qu'un feedback s'affiche

@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 :

@reibecca reibecca force-pushed the pix-13092-display-embed-into-modulix branch 3 times, most recently from 69fc22b to 33df5f9 Compare July 11, 2024 14:00
@dlahaye dlahaye force-pushed the pix-13092-display-embed-into-modulix branch 4 times, most recently from 064f846 to 113d08f Compare July 12, 2024 10:23
@reibecca reibecca force-pushed the pix-13092-display-embed-into-modulix branch 2 times, most recently from 988e900 to 4df2d80 Compare July 12, 2024 14:04
@dlahaye dlahaye force-pushed the pix-13092-display-embed-into-modulix branch from 4df2d80 to bbc60f7 Compare July 12, 2024 14:21
@dlahaye dlahaye marked this pull request as ready for review July 12, 2024 14:31
@dlahaye dlahaye requested a review from a team as a code owner July 12, 2024 14:31
Copy link
Contributor

@AnaisAllamand AnaisAllamand 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 en desktop et mobile !

@yannbertrand
Copy link
Member

Question : est-ce qu'on veut vraiment permettre d'ajouter une consigne ? Est-ce qu'un élément texte avant l'embed n'est pas suffisant ?

@dlahaye
Copy link
Contributor

dlahaye commented Jul 19, 2024

Question : est-ce qu'on veut vraiment permettre d'ajouter une consigne ? Est-ce qu'un élément texte avant l'embed n'est pas suffisant ?

@yannbertrand on en a parlé et on en a conclu que si les questions répondables possèdent leur consigne de manière intrinsèque, on pouvait le faire aussi pour les embed. Qu'en dis-tu ?

@yannbertrand
Copy link
Member

Question : est-ce qu'on veut vraiment permettre d'ajouter une consigne ? Est-ce qu'un élément texte avant l'embed n'est pas suffisant ?

@yannbertrand on en a parlé et on en a conclu que si les questions répondables possèdent leur consigne de manière intrinsèque, on pouvait le faire aussi pour les embed. Qu'en dis-tu ?

Ça a du sens, merci pour la réponse !

@yannbertrand yannbertrand force-pushed the pix-13092-display-embed-into-modulix branch from bbc60f7 to c6ed0b8 Compare July 22, 2024 07:25
@@ -5,10 +5,24 @@ import Component from '@glimmer/component';
import { tracked } from '@glimmer/tracking';
import { t } from 'ember-intl';

import didInsert from '../../../modifiers/modifier-did-insert';
Copy link
Member

Choose a reason for hiding this comment

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

Ah curieux de savoir l'histoire de ce modifier, on a une montée de version bloquée sur le même genre de sujet.

Copy link
Contributor

Choose a reason for hiding this comment

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

@yannbertrand c'est expliqué dans cette PR #9455 ;)

@reibecca reibecca force-pushed the pix-13092-display-embed-into-modulix branch from c6ed0b8 to c44f025 Compare July 22, 2024 08:47
@theotime2005 theotime2005 force-pushed the pix-13092-display-embed-into-modulix branch from c44f025 to e58eead Compare July 22, 2024 09:58
@pix-service-auto-merge pix-service-auto-merge force-pushed the pix-13092-display-embed-into-modulix branch from e58eead to 8166c38 Compare July 22, 2024 12:11
@pix-service-auto-merge pix-service-auto-merge merged commit 0a329e1 into dev Jul 22, 2024
6 of 7 checks passed
@pix-service-auto-merge pix-service-auto-merge deleted the pix-13092-display-embed-into-modulix branch July 22, 2024 12:19
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.

7 participants