-
Notifications
You must be signed in to change notification settings - Fork 17
ECHO-615 participant onboarding cards update #391
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
Conversation
- ECHO-615 remove directus call from onboarding cards
WalkthroughThis PR refactors the participant onboarding card system to derive tutorial content from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
echo/frontend/src/components/participant/hooks/useOnboardingCards.ts (1)
5-23: Slug-driven system card selection matches the new specThe refactor to
getSystemCards(lang, tutorialSlug)with a lowercase-normalized slug and an early-return on"none"cleanly encodes the new behavior:
"none"⇒ no system cards here (so participants see only the mic check flow)."basic"(and any other non-"none"slug) ⇒ privacy card + mic check from the consumer.Given the current allowed values are just
"none"and"basic", this is exactly what we want for ECHO-615. If you ever introduce additional tutorial slugs, it may be worth tightening the comment (and possibly a switch onnormalizedSlug) so future behavior is explicit rather than “anything non-none gets privacy”.echo/frontend/src/routes/participant/ParticipantStart.tsx (1)
7-48: Add defensive check before rendering ParticipantOnboardingCardsWhile the route simplification is good, the code unsafely casts
project as Projectwithout verifying it's defined. Other components usinguseParticipantProjectById(ParticipantBody, RefineSelection, ParticipantPostConversation) all defensively check or use optional chaining onprojectQuery.data.Replace
<ParticipantOnboardingCards project={project as Project} />with either:
- A defensive guard:
{project && <ParticipantOnboardingCards project={project} />}- Or move the render into a conditional block that ensures
projectis truthyThis prevents potential undefined reference errors and maintains consistency with the rest of the codebase.
echo/frontend/src/locales/fr-FR.po (1)
2171-2183: Fix a couple of French grammar nits in portal editor copyOverall the new portal/page content strings are solid, and the capitalization of “Portail” is correct for French UI per prior guidance. Two minor FR issues worth cleaning up:
Line 3419:
- Current:
Cette est une vue en direct du portail du participant.- Suggested:
Ceci est une vue en direct du portail du participant.orIl s'agit d'une vue en direct du portail du participant.Line 3451:
- Current:
Cette prompt guide comment l'IA répond aux participants.- Suggested: e.g.
Cette consigne guide la façon dont l’IA répond aux participants.orCe prompt guide la façon dont l’IA répond aux participants.Everything else in this block (“Page Content”, “Page Title”, “Portal Content”, “Thank You Page Content”, “This page is shown …”) reads naturally and keeps “Portail” capitalized as desired.
Based on learnings, this keeps French capitalization rules while tightening grammar.
Also applies to: 2435-2438, 3293-3296, 3417-3419, 3439-3445, 3450-3451, 4665-4667
echo/frontend/src/components/participant/ParticipantOnboardingCards.tsx (1)
272-371: Back button on first microphone slide is a no‑opWith the new slug mapping, when
tutorialSlug === "none"the microphone slide becomes the very first card. In that case:
- The “Back” button in the mic branch is enabled,
- But
prevSlideearly‑returns atcurrentSlideIndex === 0, so clicking Back does nothing.Not a blocker, but it’s a slightly janky UX compared to the non‑mic branch where Back is disabled on the first slide.
I’d suggest aligning behavior by disabling Back when there’s nothing to go back to:
{currentCard?.type === "microphone" ? ( <> <Button onClick={prevSlide} variant="outline" color="gray" size="md" - className="basis-1/2" + className="basis-1/2" + disabled={currentSlideIndex === 0} > <Trans id="participant.button.back.microphone">Back</Trans> </Button>This still lets people go back to Data Permission when present (basic tier), but avoids a dead button in the “none” path.
echo/frontend/src/locales/en-US.po (1)
4406-4409: Nit: tighten pronoun in “make them concrete” copyCurrent string:
msgid "participant.concrete.instructions.receive.artefact" msgstr "You'll soon get {objectLabel} to make them concrete."The plural pronoun “them” doesn’t have a clear grammatical referent here and reads a bit off.
I’d tighten this to something like:
-msgstr "You'll soon get {objectLabel} to make them concrete." +msgstr "You'll soon get {objectLabel} to make things concrete."or, if you want to lean into the feature name more directly:
-msgstr "You'll soon get {objectLabel} to make them concrete." +msgstr "You'll soon get {objectLabel} to make it concrete."Pick whichever matches the intended UX copy system-wide.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
echo/frontend/src/components/participant/ParticipantOnboardingCards.tsx(6 hunks)echo/frontend/src/components/participant/hooks/useOnboardingCards.ts(1 hunks)echo/frontend/src/components/project/ProjectPortalEditor.tsx(1 hunks)echo/frontend/src/locales/de-DE.po(35 hunks)echo/frontend/src/locales/en-US.po(36 hunks)echo/frontend/src/locales/es-ES.po(35 hunks)echo/frontend/src/locales/fr-FR.po(35 hunks)echo/frontend/src/locales/nl-NL.po(36 hunks)echo/frontend/src/routes/participant/ParticipantStart.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
echo/frontend/**/src/**/*.{css,tsx,ts}
📄 CodeRabbit inference engine (echo/frontend/AGENTS.md)
Use CSS variables
var(--app-background)andvar(--app-text)instead of hardcoded colors like#F6F4F1or#2D2D2Cto ensure theme changes propagate
Files:
echo/frontend/src/components/participant/hooks/useOnboardingCards.tsecho/frontend/src/components/participant/ParticipantOnboardingCards.tsxecho/frontend/src/components/project/ProjectPortalEditor.tsxecho/frontend/src/routes/participant/ParticipantStart.tsx
echo/frontend/**/src/{routes,components}/**/*.tsx
📄 CodeRabbit inference engine (echo/frontend/AGENTS.md)
echo/frontend/**/src/{routes,components}/**/*.tsx: Compose Mantine primitives (Stack,Group,ActionIcon, etc.) while layering Tailwind utility classes viaclassName, alongside toast feedback via@/components/common/Toaster
For Tailwind classes that need dynamic values, replace with inlinestyleprops using CSS variables instead of hardcoded hex values
Files:
echo/frontend/src/components/participant/ParticipantOnboardingCards.tsxecho/frontend/src/components/project/ProjectPortalEditor.tsxecho/frontend/src/routes/participant/ParticipantStart.tsx
echo/frontend/**/src/{theme.tsx,components,routes}/**/*.tsx
📄 CodeRabbit inference engine (echo/frontend/AGENTS.md)
When adding new Mantine components, reference the global Mantine theme in
src/theme.tsxwith customprimarycolor palette and component defaults, preferring CSS variables in styles over hardcoded hex values
Files:
echo/frontend/src/components/participant/ParticipantOnboardingCards.tsxecho/frontend/src/components/project/ProjectPortalEditor.tsxecho/frontend/src/routes/participant/ParticipantStart.tsx
echo/frontend/**/src/{components,routes}/**/*.tsx
📄 CodeRabbit inference engine (echo/frontend/AGENTS.md)
UI mutations should surface inline feedback: pair toasts with contextual Mantine
Alertcomponents inside modals/forms for errors or warnings
Files:
echo/frontend/src/components/participant/ParticipantOnboardingCards.tsxecho/frontend/src/components/project/ProjectPortalEditor.tsxecho/frontend/src/routes/participant/ParticipantStart.tsx
echo/frontend/**/src/routes/**/*.tsx
📄 CodeRabbit inference engine (echo/frontend/AGENTS.md)
Use Lingui macros
tfrom@lingui/core/macroandTransfrom@lingui/react/macrofor localizing UI strings in routed screens
Files:
echo/frontend/src/routes/participant/ParticipantStart.tsx
🧠 Learnings (12)
📓 Common learnings
Learnt from: ussaama
Repo: Dembrane/echo PR: 205
File: echo/frontend/src/lib/query.ts:1444-1506
Timestamp: 2025-07-10T12:48:20.683Z
Learning: ussaama prefers string concatenation over template literals for simple cases where readability is clearer, even when linting tools suggest template literals. Human readability takes precedence over strict linting rules in straightforward concatenation scenarios.
📚 Learning: 2025-08-08T10:39:31.114Z
Learnt from: ussaama
Repo: Dembrane/echo PR: 259
File: echo/frontend/src/components/layout/ParticipantLayout.tsx:33-33
Timestamp: 2025-08-08T10:39:31.114Z
Learning: In echo/frontend/src/components/layout/ParticipantLayout.tsx, prefer using simple pathname.includes("start") and pathname.includes("finish") to control the settings button visibility. No need to switch to segment-based matching or add a useEffect to auto-close the modal for these routes, per ussaama’s preference in PR #259.
Applied to files:
echo/frontend/src/components/participant/hooks/useOnboardingCards.tsecho/frontend/src/components/participant/ParticipantOnboardingCards.tsxecho/frontend/src/routes/participant/ParticipantStart.tsxecho/frontend/src/locales/en-US.po
📚 Learning: 2025-05-30T15:38:44.413Z
Learnt from: ussaama
Repo: Dembrane/echo PR: 169
File: echo/frontend/src/components/project/ProjectPortalEditor.tsx:409-464
Timestamp: 2025-05-30T15:38:44.413Z
Learning: Badge-based selectors in ProjectPortalEditor.tsx: Keyboard navigation enhancements for accessibility are considered optional improvements rather than critical issues. The user acknowledges these suggestions but doesn't prioritize them as blockers.
Applied to files:
echo/frontend/src/components/project/ProjectPortalEditor.tsxecho/frontend/src/locales/de-DE.poecho/frontend/src/locales/en-US.poecho/frontend/src/locales/fr-FR.po
📚 Learning: 2025-12-05T00:33:33.442Z
Learnt from: CR
Repo: Dembrane/echo PR: 0
File: echo/frontend/AGENTS.md:0-0
Timestamp: 2025-12-05T00:33:33.442Z
Learning: Applies to echo/frontend/**/src/routes/**/*.tsx : Use Lingui macros `t` from `lingui/core/macro` and `Trans` from `lingui/react/macro` for localizing UI strings in routed screens
Applied to files:
echo/frontend/src/routes/participant/ParticipantStart.tsx
📚 Learning: 2025-12-05T00:33:33.442Z
Learnt from: CR
Repo: Dembrane/echo PR: 0
File: echo/frontend/AGENTS.md:0-0
Timestamp: 2025-12-05T00:33:33.442Z
Learning: Applies to echo/frontend/**/src/routes/auth/**/*.tsx : Gentle login/logout flows use `useTransitionCurtain().runTransition()` before navigation—animations expect Directus session mutations to await that promise
Applied to files:
echo/frontend/src/routes/participant/ParticipantStart.tsx
📚 Learning: 2025-12-05T00:33:33.442Z
Learnt from: CR
Repo: Dembrane/echo PR: 0
File: echo/frontend/AGENTS.md:0-0
Timestamp: 2025-12-05T00:33:33.442Z
Learning: Applies to echo/frontend/**/src/routes/auth/**/*.tsx : Auth hero uses `/public/video/auth-hero.mp4` with `/public/video/auth-hero-poster.jpg` as poster; keep the bright blur overlay consistent when iterating on onboarding screens
Applied to files:
echo/frontend/src/routes/participant/ParticipantStart.tsx
📚 Learning: 2025-12-05T00:33:33.442Z
Learnt from: CR
Repo: Dembrane/echo PR: 0
File: echo/frontend/AGENTS.md:0-0
Timestamp: 2025-12-05T00:33:33.442Z
Learning: Applies to echo/frontend/**/src/routes/settings/**/*.tsx : Provide ergonomic navigation in settings-like routes: breadcrumb + back action (ActionIcon + navigate(-1)) with relevant iconography is the default
Applied to files:
echo/frontend/src/routes/participant/ParticipantStart.tsx
📚 Learning: 2025-12-05T00:33:33.442Z
Learnt from: CR
Repo: Dembrane/echo PR: 0
File: echo/frontend/AGENTS.md:0-0
Timestamp: 2025-12-05T00:33:33.442Z
Learning: Applies to echo/frontend/**/src/routes/auth/**/*.tsx : Auth surfaces reuse `HeaderView` by passing `isAuthenticated`/`loading` props—avoid rolling bespoke headers inside layouts
Applied to files:
echo/frontend/src/routes/participant/ParticipantStart.tsx
📚 Learning: 2025-07-10T12:47:06.269Z
Learnt from: ussaama
Repo: Dembrane/echo PR: 205
File: echo/frontend/src/locales/nl-NL.po:1246-1247
Timestamp: 2025-07-10T12:47:06.269Z
Learning: .po files (gettext internationalization files) are auto-generated by i18n tools like lingui/cli and should not be manually edited. The format of component paths and line numbers in these files is determined by the tool configuration, not manually written. Do not suggest manual edits to .po files.
Applied to files:
echo/frontend/src/locales/de-DE.po
📚 Learning: 2025-10-28T13:47:02.926Z
Learnt from: ussaama
Repo: Dembrane/echo PR: 350
File: echo/frontend/src/components/participant/ParticipantConversationText.tsx:82-85
Timestamp: 2025-10-28T13:47:02.926Z
Learning: In text mode (echo/frontend/src/components/participant/ParticipantConversationText.tsx), participants only submit PORTAL_TEXT chunks (no audio). The “Finish” button is shown only after at least one text message is saved to Directus.
Applied to files:
echo/frontend/src/locales/de-DE.poecho/frontend/src/locales/en-US.poecho/frontend/src/locales/nl-NL.poecho/frontend/src/locales/fr-FR.po
📚 Learning: 2025-09-16T08:34:38.109Z
Learnt from: ussaama
Repo: Dembrane/echo PR: 293
File: echo/frontend/src/components/chat/ChatTemplatesMenu.tsx:15-16
Timestamp: 2025-09-16T08:34:38.109Z
Learning: In ChatTemplatesMenu.tsx, titles are preferred over IDs for template selection logic since titles are unique one-liners and work effectively as identifiers. IDs were added primarily for better list rendering rather than selection purposes.
Applied to files:
echo/frontend/src/locales/en-US.po
📚 Learning: 2025-05-30T15:37:46.279Z
Learnt from: ussaama
Repo: Dembrane/echo PR: 169
File: echo/frontend/src/locales/fr-FR.po:1298-1301
Timestamp: 2025-05-30T15:37:46.279Z
Learning: In French localization files, UI terms like "Portail" should maintain their proper capitalization according to French language conventions, even if it appears inconsistent with lowercase usage in other translations. French capitalization rules for UI terms should be respected over assumed consistency patterns.
Applied to files:
echo/frontend/src/locales/fr-FR.po
🧬 Code graph analysis (1)
echo/frontend/src/components/participant/hooks/useOnboardingCards.ts (1)
echo/frontend/src/components/participant/ParticipantOnboardingCards.tsx (1)
LanguageCards(41-43)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci-check-server
🔇 Additional comments (9)
echo/frontend/src/components/project/ProjectPortalEditor.tsx (1)
523-532: Tutorial labels now accurately reflect onboarding behaviorThe updated copy for
"none"and"basic"is aligned with the new card mapping semantics (mic check only vs. privacy + mic check) and keeps the slugs stable, so everything upstream/downstream stays wired correctly. LGTM.echo/frontend/src/locales/fr-FR.po (2)
230-231: Tutorial tier labels match the new onboarding mappingAdvanced is correctly marked obsolete, and the new options:
"Basic (Data privacy + mic check)" → "Basique (Confidentialité des données + vérification du microphone)""No tutorial (only mic check)" → "Pas de tutoriel (uniquement la vérification du microphone)"read cleanly in French and line up with the ECHO-615 behavior (“basic” adds Data Permission + mic check; “none” is mic check only). Nothing to tweak here.
Also applies to: 495-501, 2070-2075
478-486: New onboarding button translations are tight and consistentThe new ids wired from
ParticipantOnboardingCards:
participant.button.back.microphone/participant.button.back→ “Retour”participant.button.continue→ “Continuer”participant.button.next→ “Suivant”participant.ready.to.begin→ “Prêt à commencer”participant.mic.check.button.skip→ “Passer”all match the UX copy and tone used elsewhere in the participant portal. These will make the new onboarding flows feel coherent across languages.
Also applies to: 709-712, 1928-1930, 2559-2560, 3062-3064
echo/frontend/src/components/participant/ParticipantOnboardingCards.tsx (1)
45-81: Tutorial‑slug–driven system cards wiring looks correctYou’ve cleanly killed the Directus dependency by:
- Shrinking props to
{ project }and- Deriving
tutorialSlug = project.default_conversation_tutorial_slug ?? "none"and feeding that intogetSystemCards(lang, tutorialSlug)for each locale.Assuming
getSystemCardstreats"none"as “no tutorial slides” and"basic"as “Data Permission + mic check”, this lines up with ECHO‑615’s mapping, while keeping the mic check + initiate slides as the universal tail. This is the right level of indirection and keeps the component dumb about tutorial tiers.Also applies to: 78-204
echo/frontend/src/locales/de-DE.po (1)
1-4000: LGTM—auto-generated .po file reflects tutorial card refactoring correctly.This is a tool-generated internationalization file (lingui/cli), so the changes here align perfectly with the backend refactoring: tutorial tier labels updated ("Basic" now references "Data privacy + mic check" instead of "Essential tutorial slides"), the "Advanced" tier removed, and card logic shifted from Directus-loaded
initialCardstoproject.default_conversation_tutorial_slug-derived cards. The line reference relocations are expected fallout from source code changes. No manual edits needed—just commit as-is.echo/frontend/src/locales/en-US.po (2)
770-773: Tutorial tier labels now explicitly communicate card behaviorThe new copy for:
"Basic (Data privacy + mic check)", and"No tutorial (only mic check)"is crisp and maps cleanly onto the new onboarding logic (basic = privacy + mic check, none = mic check only). This should make the portal editor UX much less ambiguous for project owners. LGTM.
Also applies to: 2441-2443
751-761: Participant onboarding button labels are consistent and well‑factoredThe explicit IDs for the onboarding buttons (
participant.button.back.microphone,participant.button.back,participant.button.continue,participant.button.next,participant.ready.to.begin,participant.mic.check.button.skip) line up nicely with the card flow (ready → mic check → continue/skip/back → next). Clear separation of generic vs mic-specific “Back” will also keep future refactors safe. LGTM.Also applies to: 995-1000, 2288-2291, 2861-2864, 3385-3388
echo/frontend/src/locales/nl-NL.po (2)
245-246: Removal of "Advanced" tier: LGTM.Marking this obsolete aligns with the PR objectives to restrict onboarding cards. The card mapping only specifies "none" and "basic" status, so removing "Advanced" is correct.
2193-2194: "No tutorial" update: LGTM.The label change from "only Privacy statements" to "only mic check" correctly reflects the card mapping for "none" status participants. Dutch translation is accurate.
| #: src/routes/participant/ParticipantStart.tsx:36 | ||
| msgid "An error occurred while loading the Portal. Please contact the support team." | ||
| msgstr "Ocurrió un error al cargar el Portal. Por favor, contacta al equipo de soporte." | ||
|
|
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.
🧹 Nitpick | 🔵 Trivial
Spanish onboarding + portal strings are aligned with the new flow
Spot‑checks on the new/updated entries look solid:
An error occurred while loading the Portal…→ “Ocurrió un error al cargar el Portal. Por favor, contacta al equipo de soporte.” is correct and matches the new error path inParticipantStartRoute.- Tutorial option labels:
"Basic (Data privacy + mic check)"→ “Básico (Privacidad de datos + verificación de micrófono)”."No tutorial (only mic check)"→ “Sin tutorial (solo verificación de micrófono)”.
These mirror the new"basic"/"none"semantics for onboarding cards.
- “Make it concrete” copy (
Concrete Topics, “At least one topic must be selected to enable Make it concrete”) is translated consistently and keeps the feature name recognizable. - New participant onboarding controls (
participant.button.back*,participant.button.continue,participant.button.next,participant.ready.to.begin,participant.mic.check.button.skip) all have idiomatic, context-appropriate Spanish.
Only minor note for later: the dashboard.dembrane.concrete.* explicit‑id entries still use the key as the msgstr, which likely predates this PR and can be cleaned up separately if you want fully localized concrete-feature copy.
Also applies to: 495-502, 656-659, 2070-2076, 477-487, 2559-2562, 3062-3065
| msgid "Basic (Data privacy + mic check)" | ||
| msgstr "Basis (Datenschutz + microfoon-check)" |
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.
Critical: German word in Dutch translation.
Line 551 uses "Datenschutz" which is German, not Dutch. This breaks the nl-NL locale.
Apply this diff to fix the translation:
-msgstr "Basis (Datenschutz + microfoon-check)"
+msgstr "Basis (Dataprivacy + microfoon-check)"Alternative correct Dutch terms:
- "Gegevensbescherming" (formal)
- "Privacy van gegevens" (descriptive)
Choose based on your preferred tone, but "Dataprivacy" is commonly used in Dutch tech UIs.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| msgid "Basic (Data privacy + mic check)" | |
| msgstr "Basis (Datenschutz + microfoon-check)" | |
| msgid "Basic (Data privacy + mic check)" | |
| msgstr "Basis (Dataprivacy + microfoon-check)" |
🤖 Prompt for AI Agents
In echo/frontend/src/locales/nl-NL.po around lines 550-551, the Dutch
translation incorrectly contains the German word "Datenschutz"; update the
msgstr to use a proper Dutch term (e.g., replace "Datenschutz + microfoon-check"
with "Dataprivacy + microfoon-check" or "Gegevensbescherming + microfoon-check")
so the line reads: msgstr "Basis (Dataprivacy + microfoon-check)" (or your
chosen Dutch alternative).
Summary by CodeRabbit
New Features & Updates
Refactor
Localization
✏️ Tip: You can customize this high-level summary in your review settings.