-
Notifications
You must be signed in to change notification settings - Fork 7
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
feat(conversation): #WB-3833 outbox details in message list #607
Conversation
* feat(conversation): WB-3466 react migration init * update build react node * update message * chore: migration, clean up unnecessary files * chore: migration, update additional files * chore: update .gitignore * chore: update .gitignore and build scripts * feat(conversation): #WB-3466, GET /conversation/conversation renderView * chore: some frontend files must not be versioned * chore: generic build.sh frontend build --------- Co-authored-by: RomuC <romuald.coulaud@gmail.com>
* chore: fix build scripts and POMs * chore: added a buildBackend option to build.sh * chore: rename gradle.properties files * feat(conversation): #WB-3630, added GET /conversation/api/* endpoints * feat(conversation, directory): #WB-3630, implement GET /conversation/api/folders/:folderId/messages * feat(conversation): #WB-3630, implement GET git /conversation/api/folders * feat(conversation): #WB-3630, implement GET /conversation/api/messages/{messageId} and cleanup code * Revert "chore: rename gradle.properties files" This reverts commit afa0667. * chore: feedback from PR * fix: feedback from PR
* fix : WB-3640, create single HAS_FUNCTION link when merging two admls * fix : WB-3640, adding integration test * fix : WB-3640, forget test invocation * fix : WB-3640, isolate and fix test case * fix : WB-3640, enrich test cases
…ges of React app (#562) * feat(conversation): #WB-3633, initial commit * wip * wip * wip * fix: rename routes * wip * fix: lint error * fix: redirection for hash-based folders URLs * feat: create folder and message API service factories * wip: folder and message queries * wip: message queries + simplifications * fix: feedback from PR * fix: feedback from PR * fix: feedback from PR * feat: folderService tests * feat: messageService tests
…/apps with unexpected empty content + some refactoring for better code readability : - remove async keyword in the ApplicationController DI - build the scope THEN apply final checks and async rendering - try/catch some potential errors
* feat(conversation): #WB-3428, left menu navigation * wip * wip * Desktop Menu with counters * Wip: Mobile Menu with counters * wip * feat: Mobile Menu with counters and navigation * wip * feat(conversation): actions on folders * fix: feedback from PR
…ds, when getting a message in details in API v1+
* fix: WB-3666, optimize voluminous shares * fix: WB-3666, fix after review * fix: WB-3666, shares optims parameterizable
…ons, search an… (#569) * feat: #WB-3426 #WB-3462 #WB-3735 message list with actions, search and filter
@@ -39,12 +39,12 @@ export function MessageHeader({ message }: MessageHeaderProps) { | |||
</a> | |||
<em className="text-gray-700">{fromNow(date)}</em> | |||
</div> | |||
<MessageRecipientList label={t("at")} recipients={to} /> | |||
<MessageRecipientList head={<b>{t("at")}</b>} recipients={to} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK pour le passage de ReactElement
via des props, car l'usage reste simple et interne à la messagerie.
Mais sinon, on décompose habituellement en sous-composants que l'on passe comme children pour leur rendu.
Ici cela donnerait par exemple :
<MessageRecipientList recipients={to}>
<MessageRecipientList.Head>{t("at")}</MessageRecipientList.Head>
</MessageRecipientList>
Voir un exemple d'implémentation avec le composant Dropdown ici.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
top merci pour l'info ! 🙏
|
||
render(<MessagePreview message={message} />); | ||
|
||
const toLabel = screen.getByText('at'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getByText
fonctionne ? Jusqu'à présent, seules les fonctions findBy***
ont fonctionné pour moi.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On avait refait un tour avec @Romu-C et on pourra discuter de ça en DST :
getByText ⇒ synchrone + renvoi une erreur si trouve pas
queryByText ⇒ synchrone + renvoi null si trouve pas
findByText ⇒ asynchrone + renvoi une erreur si trouve pas
donc si pas besoin d'attendre on peut utiliser getByText
. C'est le cas pour ce composant normalement
export function MessagePreview({ message }: MessagePreviewProps) { | ||
const { t } = useTranslation('conversation'); | ||
|
||
const { folderId } = useParams<{ folderId: MessageFolderId }>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Le typage en MessageFolderId
est faux à mon avis. Il s'agit simplement d'une string
.
Notamment lorsqu'on previsualise un message déplacé dans un dossier utilisateur (l'URL est du genre https://recette-ode1.opendigitaleducation.com/conversation/folder/6f40cac8-7435-4700-980c-db752a58c1b2/....
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
j'avais fait ça pour faciliter le dev, mais en effet, ce n'est pas toujours vrai !
import { Fragment } from 'react/jsx-runtime'; | ||
import { useI18n } from '~/hooks'; | ||
import { Recipients } from '~/models'; | ||
|
||
export interface RecipientListProps { | ||
recipients: Recipients | ||
label: string | ||
head: ReactElement | string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Je suis étonné qu'il n'y ait pas de caractère ;
à la fin de la ligne.
As-tu activé le plugin prettier
dans ton IDE ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bien vu ! ohlala, j'avais des conflits de code formater, je viens de faire le menage, mais j'ai dû reformater tout le code que j'ai créé. Merci
7e92d6b
to
73dd077
Compare
Description
Détails propres aux messages envoyés dans la liste des messages :
Fixes
(Enter here Jira or Redmine ticket(s) links)
Type of change
Please check options that are relevant.
Which packages changed?
Please check the name of the package you changed
Tests
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 ! 😃