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

feat(conversation): #WB-3833 outbox details in message list #614

Merged
merged 14 commits into from
Mar 7, 2025
Merged

feat(conversation): #WB-3833 outbox details in message list #614

merged 14 commits into from
Mar 7, 2025

Conversation

damienromito
Copy link

@damienromito damienromito commented Mar 4, 2025 •
edited
Loading

Description

PR d'origine : #607
Détails propres aux messages envoyés dans la liste des messages :

  • précise le destinataire au lieu de l’expéditeur, préfixé par “À”.
  • précise l'avatar du destinataire au lieu de celui de l’expéditeur.
  • tronque la liste des destinataires sur une seule ligne dans la vue “liste”, mais pas dans les détails d’un message (tout afficher).
  • affiche une icône spécifique s'il y a plusieurs destinataires, au lieu de l'avatar.
  • dans la boite de reception afficher “moi” si user actif
  • afficher tous les destinataires ( “to” “cc” et “cci”) dans le champs l'aperçu des destinataires

Fixes

(Enter here Jira or Redmine ticket(s) links)

Type of change

Please check options that are relevant.

  • Chore (PATCH)
  • Doc (PATCH)
  • Bug fix (PATCH)
  • New feature (MINOR)

Which packages changed?

Please check the name of the package you changed

  • admin
  • app-registry
  • archive
  • auth
  • cas
  • common
  • communication
  • conversation
  • directory
  • feeder
  • infra
  • portal
  • session
  • test
  • tests
  • timeline
  • workspace

Tests

  • should display "to" label and recipient name when is in outbox
  • should display recipient avatar when is in outbox
  • should display group avatar icon when more than one recipient in outbox

Reminder

  • Security flaws

  • Performance impacts (think bulk !)

  • Unit tests were replayed

  • Unit tests were added and/or changed

  • I have updated the reminder for the version including my modifications

  • All done ! 😃

@damienromito damienromito changed the base branch from master to develop-b2school March 4, 2025 16:00
@damienromito damienromito marked this pull request as ready for review March 4, 2025 16:06
damienromito
damienromito commented Mar 4, 2025
View reviewed changes
conversation/frontend/src/features/Message/message-preview.tsx Outdated
<IconGroupAvatar
className="w-16"
aria-label={t('recipient.avatar.group')}
role="img"
Copy link
Author

Choose a reason for hiding this comment

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

j'ai un doute sur l'utilité de ces balise pour l'accessibilité, quand j'affiche l'accessibility tree, le composant n'apparait pas. Qu'en pensez-vous ?

Copy link
Contributor

Choose a reason for hiding this comment

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

J'avoue je ne sais pas trop mais j'ai un doute aussi.

Copy link
Contributor

Choose a reason for hiding this comment

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

L'attribut role est propagé dans le HTML généré au final ? C'est React qui s'en charge ?

Copy link
Author

Choose a reason for hiding this comment

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

En fait l'attribut role n'a pas été prévu sur le composant , normal, donc si je l'ai mis sur la div, ça fonctionne bien

@damienromito damienromito self-assigned this Mar 4, 2025
jcbe-ode
jcbe-ode reviewed Mar 4, 2025
View reviewed changes
conversation/backend/src/main/resources/i18n/fr.json
@@ -7,7 +7,6 @@
"author.avatar": "Avatar de l'auteur",
"cancel": "Annuler",
"cc": "Cc :",
"cci": "Cci :",
Copy link
Contributor

Choose a reason for hiding this comment

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

Bizarre que "Cci" ne soit plus utilisé du tout, non ?
Je pense que Romuald va en avoir besoin pour la page de création d'un message :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Oui, il sera utile plus tard.

Copy link
Author

Choose a reason for hiding this comment

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

c'est moi qui l'avait ajouter, donc je l'ai suppr. On pourra l'ajouter lorsque ça sera utile en effet

Copy link
Author

Choose a reason for hiding this comment

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

en fait on va l'afficher quand l'utilisateur actif s'est envoyé à lui même un message en cci 😆

conversation/frontend/src/features/Message/message-preview.tsx
variant="circle"
/>

{'outbox' === folderId ? (
Copy link
Contributor

Choose a reason for hiding this comment

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

On a ici de la logique métier, qui fait le rendu

  • d'un autre composant contenant aussi de la logique métier,
  • ou le rendu d'un avatar simple.

Du coup, ça vaut le coup de créer un hook contenant toute la logique métier, lequel retourne les données pré-machées, puis d'injecter ces données en props dans le composant RecipientAvatar
Lui ne ferait ensuite que le rendu à l'écran du ou des avatars, selon le cas.

Maintenant, si cela doit faire ré-écrire les tests associés, ça ne vaut peut-être plus autant le coup...

conversation/frontend/src/features/Message/message-preview.tsx
<div className="d-flex flex-fill flex-column overflow-hidden">
<div className="d-flex flex-fill justify-content-between overflow-hidden">
<div className="text-truncate flex-fill">
{message.from.displayName}
{'outbox' === folderId ? (
Copy link
Contributor

Choose a reason for hiding this comment

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

Idem

@damienromito damienromito requested a review from jcbe-ode March 6, 2025 16:21
@damienromito damienromito merged commit 8152bd5 into develop-b2school Mar 7, 2025
Romu-C pushed a commit that referenced this pull request Mar 7, 2025
@damienromito @jcbe-ode @Romu-C
feat(conversation): #WB-3833 outbox details in message list (#614)
577008b 
* chore: prepare develop-b2school version

* fix(conversation):  #WB-3833  add "at" before de recipient name

* fix(conversation):  #WB-3833  general fix nosubject text coloe

* fix(conversation):  #WB-3833  display recipient avatar in outbox

* fix(conversation):  #WB-3833  truncate recipient list only on message list

* fix(conversation):  #WB-3833  remove MessageFolderId type

* fix(conversation):  #WB-3833  setup code format

* fix(conversation):  #WB-3833  option to disable link on recipientList

* fix(conversation):  #WB-3833  remove cci recipients - thx Loic

* feat(conversation):  #WB-3833  add avatar group icon

* refactor(conversation):  #WB-3833  split logic from interface and apply best practices

* feat(conversation): in outbox list, display all recipients (to, cc, cci)

* fix(conversation): file naming

---------

Co-authored-by: Jean-Christophe <jean-christophe.benoit@opendigitaleducation.com>
Romu-C pushed a commit that referenced this pull request Mar 7, 2025
@damienromito @jcbe-ode @Romu-C
feat(conversation): #WB-3833 outbox details in message list (#614)
198e0b3 
* chore: prepare develop-b2school version

* fix(conversation):  #WB-3833  add "at" before de recipient name

* fix(conversation):  #WB-3833  general fix nosubject text coloe

* fix(conversation):  #WB-3833  display recipient avatar in outbox

* fix(conversation):  #WB-3833  truncate recipient list only on message list

* fix(conversation):  #WB-3833  remove MessageFolderId type

* fix(conversation):  #WB-3833  setup code format

* fix(conversation):  #WB-3833  option to disable link on recipientList

* fix(conversation):  #WB-3833  remove cci recipients - thx Loic

* feat(conversation):  #WB-3833  add avatar group icon

* refactor(conversation):  #WB-3833  split logic from interface and apply best practices

* feat(conversation): in outbox list, display all recipients (to, cc, cci)

* fix(conversation): file naming

---------

Co-authored-by: Jean-Christophe <jean-christophe.benoit@opendigitaleducation.com>
@damienromito damienromito deleted the feat/WB-3833-list-messages-sent-rebased branch March 10, 2025 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Romu-C Romu-C Romu-C approved these changes

@evanthonyly evanthonyly Awaiting requested review from evanthonyly

@jcbe-ode jcbe-ode Awaiting requested review from jcbe-ode

Assignees

@damienromito damienromito

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@damienromito @Romu-C @jcbe-ode