Skip to content

Option in Periods Group that allows users to create their own custom periods template#1335

Draft
minh-de-rien wants to merge 10 commits intochairemobilite:mainfrom
Abdel667:custom-periods-interface
Draft

Option in Periods Group that allows users to create their own custom periods template#1335
minh-de-rien wants to merge 10 commits intochairemobilite:mainfrom
Abdel667:custom-periods-interface

Conversation

@minh-de-rien
Copy link
Contributor

@minh-de-rien minh-de-rien commented Apr 14, 2025

Add Custom Periods Template Functionality in Period Groups Dropdown

This feature allows users to create, edit, and save their own custom period templates within the Period Groups dropdown.

Features Implemented

  • New dropdown option: Added a Add custom periods entry to the Period Groups dropdown to begin the template creation.
  • Custom period management:
    • Users can add, remove, and update custom periods.
    • Users can manually input start and end times by clicking on the displayed time, which focuses the input field.
  • Template saving:
    • Users can name and save their custom period configuration as a reusable template.
    • Saved templates are updated in the dropdown (note: currently, the newly saved template will only appear after a manual refresh or when switching services. I'm not sure how to fix this)
  • New components and updates:
    • Created InputModal.tsx to handle template naming.
    • Updated TransitScheduleEdit.tsx with functions:
      • addCustomPeriod
      • removeCustomPeriod
      • savePeriodTemplate
      • deletePeriodTemplate
    • Updated TransitSchedulePeriod.tsx from line 158 to 190. When clicking on the time label on the left side, the corresponding input field is automatically focused for editing (note: included here but also modified in the BatchSchedules PR — merge conflict to be resolved later).

Notes

  • User Periods templates are currently saved in localStorage. We should maybe save them in the database in the future.
  • The name of each period is imposed and not customizable at the moment.
  • The two generic functions open/closeModal in SaveableObjectForm.tsx, if approuved, are intended to replace the more specific open/closeDeleteConfirmModal and open/closeBackConfirmModal. All calls to the existing functions should be updated eventually.

Screenshot 2025-04-14 164029

@minh-de-rien minh-de-rien force-pushed the custom-periods-interface branch from 0b2e1fc to a5a23a6 Compare April 15, 2025 18:32
@minh-de-rien minh-de-rien force-pushed the custom-periods-interface branch from eee268e to 8373127 Compare April 15, 2025 21:39
autocompleteChoices?: ({ label: string; value: string } | string)[];
}

const InputModal: React.FC<InputModalProps> = ({
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pourquoi un nouveau composant InputModal. Que manquait-il aux autres modal? ça prendrait une explication de pourquoi ce type est nécessaire

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Lorsqu'on clique sur le bouton Sauvegarder comme modèle, notre idée est de permettre à l’utilisateur de nommer le modèle. Alors, je avais pensé qu’on pourrait créer une fenêtre modale qui combine les fonctionnalités de InputString.tsx et de ConfirmModal.tsx, pour que l’utilisateur puisse entrer le nom directement. Est-ce que vous pensez que c'est nécessaire? On pourrait également mettre un champs quelque part sur l'interface de modification de l'horaire directement et utiliser InputString, mais je pense qu'il serait peut-être moins intuitif que d'avoir une fenêtre de confirmation.

image

});
};

protected openModal = (modalName: keyof this['state']) => (e?: any) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Est-il nécessaire de faire ce changement dans cette classe de base? Plutôt que dans la classe qui l'utilise? On va généralement privilégier d'attendre d'avoir au moins 2 ou 3 utilisations d'un feature avant de l'emmener dans les classes génériques (sinon, ça devient comme un API et il faut tester/supporter, etc)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

En fait, au départ, j'avais écrit ces fonctions dans le fichier TransitScheduleEdit.tsx pour changer l'état du InputModal qu'on a créé. Mais en fouillant, j'avais trouvé qu'on avait déjà deux fonctions qui se ressemblait beaucoup openDeleteConfirmModal et openBackConfirmModal (idem à pour les fonctions de close). Alors, comme j'avais aussi besoin d'ovrir et de fermer la fenêtre InputModal, je me suis dit que je peux réutiliser ces fonctions de open et de close, avec une petite modification pour qu'elles soient plus génériques.
Je peux remettre les fonctions initiales dans TransitScheduleEdit sans problème!

}}
>
{!noBlank && <option key={'_blank'} value=""></option>}
{!noBlank && <option value=""></option>}
Copy link
Collaborator

Choose a reason for hiding this comment

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

why remove the key here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oups! Je l'ai enlevé sans faire exprès. Je vais le remettre!

// TODO: The name of each period is imposed and not customizable at the moment
const newPeriod = {
shortname: `custom_period_${nextPeriodId}`,
name: {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ici, est-ce que la structure de la période impose d'avoir un object avec fr et en comme clés? Ou peut-on utiliser des strings de translation t('period')? Si c'est imposé fr/en, svp, ajouter un commentaire avec un FIXME pour le jour où on supportera la customization du nom

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oui en fait, la structure de newPeriod suit la logique de defaultPreferences.config.ts. Donc, on accède au nom de la période avec periodsGroup.name[langue]. Puisque name est un objet bilingue, j'ai ajouté un nom de période par défaut pour les deux langues. Je ne suis pas sure si c'est la bonne pratique. Je crois que si on veut utiliser t('period'), cela requiert une refactorisation de la liste des groupes périodes par défault.

image

if (!p.name) p.name = { en: p.shortname, fr: p.shortname };
});

// TODO: User Periods templates are currently saved in localStorage
Copy link
Collaborator

Choose a reason for hiding this comment

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

Est-ce que les sauvegarder dans les préférences utilisateur est suffisant? ça serait portable dans tous les ordis où cet utilisateur utiliserait Transition. Et qu'arrive-t-il si un autre utilisateur ouvre l'horaire avec périodes personnalisées généré par qqn d'autre? Cet autre utilisateur pourra-t-il mettre à jour l'horaire avec ces mêmes périodes ou ça ne fonctionne pas du tout?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ohh je n'ai pas pensé à ça! En ce moment, la sauvegarde de localStorage est local à l'appareil et au navigateur. Il n’est pas partagé entre utilisateurs, ni entre machines. Je pense sauvegarder les périodes dans la base de données dans un deuxième temps, mais je ne suis pas sure pour le comportement désirer si un autre utilisateur utilise le même modèle de périodes. En ce moment, les horaires / les services créés par un utilisateur sont-ils accessibles par un autre?

@@ -0,0 +1,350 @@
import React from 'react';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ajouter le copyright header

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants