Skip to content

Feat/improvements#5

Merged
Kaiohz merged 2 commits into
mainfrom
feat/improvements
Apr 23, 2026
Merged

Feat/improvements#5
Kaiohz merged 2 commits into
mainfrom
feat/improvements

Conversation

@Kaiohz
Copy link
Copy Markdown
Contributor

@Kaiohz Kaiohz commented Apr 13, 2026

No description provided.

@Kaiohz Kaiohz changed the base branch from BRIC-4/init-composable-ui to main April 13, 2026 09:00
Copy link
Copy Markdown
Contributor Author

@Kaiohz Kaiohz left a comment

Choose a reason for hiding this comment

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

🟢 Review: Feat/improvements — Score 7/10

✅ Points positifs

  1. Fix dialog accessibility : Remplacement de <dialog open role="none"> par <div role="document"> — correct, le <dialog> HTML natif a un comportement de focus/overlay qui entre en conflit avec l'overlay custom. Meilleure sémantique.

  2. Fix scroll dans modals : overflow-y-auto sur le conteneur + items-start pt-[10vh] au lieu de items-center — quand le contenu dépasse, la modal scroll au lieu d'être coupée. Bon fix UX.

  3. Toast d'erreur : Ajout de toast.error() dans ChatInput et useStreamChat — l'utilisateur a enfin un feedback visuel quand un message ou un stream échoue.

  4. Race condition fix dans ChatInput : await queryClient.invalidateQueries() AVANT de clear le pending state — avant, le invalidate était synchrone (non-awaited) et le clear pouvait arriver avant le refetch, causant un flash vide.

  5. Timeout API : chatApi.postMessage passe à 5min (300000ms) — nécessaire pour les agents LLM longs.

⚠️ Points d'attention

  1. Timeout hardcodé : 300000 est en dur dans l'appel API. Devrait être configurable via env var ou constante (les agents lents vs rapides n'ont pas le même besoin).

  2. useStreamChat : finally sans catch : Le try { await invalidate } finally { clear state } est bien, mais si invalidateQueries throw pour une raison inattendue, le state sera quand même nettoyé — c'est le comportement voulu, mais pas de logging de l'erreur.

  3. Pas de référence Jira ni description dans la PR

  4. Pas de tests pour les changements (toast, dialog, timeout) — comprendre pour du UI, mais les hooks useStreamChat et useSendMessage pourraient avoir des tests unitaires

💡 Suggestions

  • Extraire le timeout dans une constante const API_TIMEOUT_MS = 300_000; ou env var VITE_API_TIMEOUT
  • Ajouter description + référence Jira
  • Considérer des tests pour useStreamChat (error callback, finally behavior)

Review by SoluBot 🤖

@Kaiohz
Copy link
Copy Markdown
Contributor Author

Kaiohz commented Apr 14, 2026

Code Review — Feat/improvements

✅ Points positifs

  • Fix accessibility majeur : remplacement <dialog> par <div role="document">
  • Scroll fix : overflow-y-auto + pt-[10vh] — les modals ne débordent plus
  • Changement minimal, impact UX maximal — exactement le genre de fix qui améliore l'expérience sans risque de régression

Score : 8/10 — LGTM 🚀

@Kaiohz Kaiohz merged commit 2ab1685 into main Apr 23, 2026
1 check failed
@Kaiohz Kaiohz deleted the feat/improvements branch April 23, 2026 17:33
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.

1 participant