Conversation
WalkthroughComprehensive feature update implementing anonymized content visualization and UI refinements. Adds a new Directus folder configuration with expanded permissions, introduces a RedactedText utility component for displaying anonymized data with tooltips, integrates it across multiple transcript-rendering components, replaces legacy shield/logo icons with detective and rosette discount icons, and converts whitelabel logo upload from manual to automatic-on-selection. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
echo/directus/sync/collections/folders.json (1)
2-21:⚠️ Potential issue | 🟠 MajorThree
custom_logosfolder entries with distinct_syncIds — looks like accidental dupes, ship it or nuke two of them.You've got three separate entries all named
"custom_logos"(lines 2-6, 12-16, 17-21) each with a unique_syncId. On sync this will create three identical folders in Directus. Guessing only one is intentional — the other two are leftover from prior attempts or merge artifacts.🔧 Keep only one entry
[ { "name": "custom_logos", "parent": null, "_syncId": "416965c6-7695-4235-8322-8515c9a05820" }, { "name": "Public", "parent": null, "_syncId": "74232676-80e7-4f8c-8012-c0d59e6d0a24" - }, - { - "name": "custom_logos", - "parent": null, - "_syncId": "9358abf8-aa7d-464e-9ea5-5e998f3cb807" - }, - { - "name": "custom_logos", - "parent": null, - "_syncId": "eabbb779-e43b-4c78-baa5-5a5cd102a596" } ]echo/frontend/src/components/settings/WhitelabelLogoCard.tsx (1)
54-54:⚠️ Potential issue | 🟡 MinorUI copy uses "successfully" — guidelines say nope.
Per your style guide, never use "successfully" in user-facing text. Keep it tight.
✏️ Fix
- toast.success(t`Logo updated successfully`); + toast.success(t`Logo updated`);As per coding guidelines: "never use 'successfully'" for user-facing text in
echo/frontend/src/**/*.{ts,tsx}.
🤖 Fix all issues with AI agents
In `@echo/frontend/src/components/common/RedactedText.tsx`:
- Around line 86-93: The conditional in the RedactedText component around the
`rendered` value is dead — both branches return identical JSX — so remove the
`if (typeof rendered === "string")` check and simplify the render to a single
return that outputs `<span className={className}>{rendered}</span>`; update the
component using `useMemo(() => parseRedactedText(children), [children])` (and
keep `rendered`, `className`, `children`, and `parseRedactedText` references
intact) to avoid duplicated markup and improve clarity.
- Around line 7-17: REDACTED_LABELS contains user-visible label strings that are
not passed through the Lingui t macro, so extractable translations are missed;
update the code so labels are generated at render-time via t (e.g., replace the
static REDACTED_LABELS map with a function getRedactedLabel(key: string) that
returns t`...` for each label or move the mapping logic into
formatLabel/component rendering and call t for each user-facing string),
ensuring every label (Address, Email, Card, IBAN, ID, License Plate, Name,
Phone, Username) is wrapped with t so Lingui can extract them.
In `@echo/frontend/src/components/participant/UserChunkMessage.tsx`:
- Around line 105-116: The JSX currently renders both the "Transcription in
progress" and an empty <Markdown> because the ternary checks
chunk.transcript?.includes(...) after chunk.transcript == null; change the
conditional flow in UserChunkMessage (around the Paper/Text block) to an if/else
chain: if chunk.transcript == null render the Markdown with "*Transcription in
progress.*" and return/skip the other branch; else if
chunk.transcript.includes("<redacted_") render
<RedactedText>{chunk.transcript}</RedactedText>; otherwise render <Markdown
content={chunk.transcript} /> so only one of the three render paths
(in-progress, redacted, or transcript) runs.
In `@echo/frontend/src/components/project/ProjectPortalEditor.tsx`:
- Around line 1223-1226: Replace the hardcoded Tailwind color class on the
IconInfoCircle (remove className="text-gray-400") and use the same theme-aware
pattern as other icons by passing the color via the inline prop (e.g.,
color="var(--mantine-color-primary-filled)") so the icon follows CSS variables
and theme changes; update the IconInfoCircle element to use that color prop and
remove the static utility class.
| const REDACTED_LABELS: Record<string, string> = { | ||
| address: "Address", | ||
| card: "Card", | ||
| email: "Email", | ||
| iban: "IBAN", | ||
| id: "ID", | ||
| license_plate: "License Plate", | ||
| name: "Name", | ||
| phone: "Phone", | ||
| username: "Username", | ||
| }; |
There was a problem hiding this comment.
Label strings aren't wrapped with t — won't be extracted for localization.
Since the localization workflow is active, these user-visible labels ("Address", "Email", etc.) should go through Lingui so translators can pick them up.
✏️ Example fix
+import { t } from "@lingui/core/macro";
+
const REDACTED_LABELS: Record<string, string> = {
- address: "Address",
- card: "Card",
- email: "Email",
+ address: t`Address`,
+ card: t`Card`,
+ email: t`Email`,
// ... etc
};Note: since t is a macro evaluated at render time, you'd need to make this a function or compute labels inside the component/badge. Alternatively, move formatLabel to use t dynamically.
As per coding guidelines: "Localization workflow is active: keep Lingui extract/compile scripts in mind when touching t/Trans strings."
🤖 Prompt for AI Agents
In `@echo/frontend/src/components/common/RedactedText.tsx` around lines 7 - 17,
REDACTED_LABELS contains user-visible label strings that are not passed through
the Lingui t macro, so extractable translations are missed; update the code so
labels are generated at render-time via t (e.g., replace the static
REDACTED_LABELS map with a function getRedactedLabel(key: string) that returns
t`...` for each label or move the mapping logic into formatLabel/component
rendering and call t for each user-facing string), ensuring every label
(Address, Email, Card, IBAN, ID, License Plate, Name, Phone, Username) is
wrapped with t so Lingui can extract them.
| }) => { | ||
| const rendered = useMemo(() => parseRedactedText(children), [children]); | ||
|
|
||
| if (typeof rendered === "string") { | ||
| return <span className={className}>{rendered}</span>; | ||
| } | ||
|
|
||
| return <span className={className}>{rendered}</span>; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Dead branch — both paths return the same JSX.
The typeof rendered === "string" check differentiates nothing here since both branches render identical markup. Simplify.
♻️ Simplify
export const RedactedText = ({
children,
className,
}: {
children: string;
className?: string;
}) => {
const rendered = useMemo(() => parseRedactedText(children), [children]);
-
- if (typeof rendered === "string") {
- return <span className={className}>{rendered}</span>;
- }
-
return <span className={className}>{rendered}</span>;
};🤖 Prompt for AI Agents
In `@echo/frontend/src/components/common/RedactedText.tsx` around lines 86 - 93,
The conditional in the RedactedText component around the `rendered` value is
dead — both branches return identical JSX — so remove the `if (typeof rendered
=== "string")` check and simplify the render to a single return that outputs
`<span className={className}>{rendered}</span>`; update the component using
`useMemo(() => parseRedactedText(children), [children])` (and keep `rendered`,
`className`, `children`, and `parseRedactedText` references intact) to avoid
duplicated markup and improve clarity.
| <Paper className="my-2 rounded-t-xl rounded-bl-xl border-0 bg-gray-100 p-4"> | ||
| <Text className="prose text-sm"> | ||
| {chunk.transcript == null && ( | ||
| <Markdown content={t`*Transcription in progress.*`} /> | ||
| )} | ||
| {chunk.transcript?.includes("<redacted_") ? ( | ||
| <RedactedText>{chunk.transcript}</RedactedText> | ||
| ) : ( | ||
| <Markdown content={chunk.transcript ?? ""} /> | ||
| </Text> | ||
| </Paper> | ||
| )} | ||
| </Text> | ||
| </Paper> |
There was a problem hiding this comment.
Bug: double-render when transcript is null — both "Transcription in progress" AND an empty <Markdown> will render.
When chunk.transcript is null, the first condition (line 107) renders the progress message. But the ternary on line 110 also evaluates: null?.includes(...) is undefined (falsy), so it falls through to <Markdown content="" />. You get two elements rendered.
Need a proper if/else chain or early return.
🐛 Proposed fix
<Paper className="my-2 rounded-t-xl rounded-bl-xl border-0 bg-gray-100 p-4">
<Text className="prose text-sm">
- {chunk.transcript == null && (
+ {chunk.transcript == null ? (
<Markdown content={t`*Transcription in progress.*`} />
- )}
- {chunk.transcript?.includes("<redacted_") ? (
+ ) : chunk.transcript.includes("<redacted_") ? (
<RedactedText>{chunk.transcript}</RedactedText>
) : (
<Markdown content={chunk.transcript ?? ""} />
)}
</Text>
</Paper>📝 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.
| <Paper className="my-2 rounded-t-xl rounded-bl-xl border-0 bg-gray-100 p-4"> | |
| <Text className="prose text-sm"> | |
| {chunk.transcript == null && ( | |
| <Markdown content={t`*Transcription in progress.*`} /> | |
| )} | |
| {chunk.transcript?.includes("<redacted_") ? ( | |
| <RedactedText>{chunk.transcript}</RedactedText> | |
| ) : ( | |
| <Markdown content={chunk.transcript ?? ""} /> | |
| </Text> | |
| </Paper> | |
| )} | |
| </Text> | |
| </Paper> | |
| <Paper className="my-2 rounded-t-xl rounded-bl-xl border-0 bg-gray-100 p-4"> | |
| <Text className="prose text-sm"> | |
| {chunk.transcript == null ? ( | |
| <Markdown content={t`*Transcription in progress.*`} /> | |
| ) : chunk.transcript.includes("<redacted_") ? ( | |
| <RedactedText>{chunk.transcript}</RedactedText> | |
| ) : ( | |
| <Markdown content={chunk.transcript ?? ""} /> | |
| )} | |
| </Text> | |
| </Paper> |
🤖 Prompt for AI Agents
In `@echo/frontend/src/components/participant/UserChunkMessage.tsx` around lines
105 - 116, The JSX currently renders both the "Transcription in progress" and an
empty <Markdown> because the ternary checks chunk.transcript?.includes(...)
after chunk.transcript == null; change the conditional flow in UserChunkMessage
(around the Paper/Text block) to an if/else chain: if chunk.transcript == null
render the Markdown with "*Transcription in progress.*" and return/skip the
other branch; else if chunk.transcript.includes("<redacted_") render
<RedactedText>{chunk.transcript}</RedactedText>; otherwise render <Markdown
content={chunk.transcript} /> so only one of the three render paths
(in-progress, redacted, or transcript) runs.
| <IconInfoCircle | ||
| size={20} | ||
| className="text-gray-400" | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use a CSS variable instead of text-gray-400 for theme consistency.
Every other icon in this file uses color="var(--mantine-color-primary-filled)" via an inline style prop. This one breaks the pattern with a hardcoded Tailwind color class, which won't respect theme changes.
Proposed fix
<IconInfoCircle
size={20}
- className="text-gray-400"
+ style={{ color: "var(--mantine-color-dimmed)" }}
/>As per coding guidelines: "Keep static utility classes (borders, spacing, layout) in Tailwind; move theme-dependent colors to CSS variables" and "Use var(--app-background) and var(--app-text) instead of hardcoded colors like #F6F4F1 or #2D2D2C to ensure theme changes propagate".
📝 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.
| <IconInfoCircle | |
| size={20} | |
| className="text-gray-400" | |
| /> | |
| <IconInfoCircle | |
| size={20} | |
| style={{ color: "var(--mantine-color-dimmed)" }} | |
| /> |
🤖 Prompt for AI Agents
In `@echo/frontend/src/components/project/ProjectPortalEditor.tsx` around lines
1223 - 1226, Replace the hardcoded Tailwind color class on the IconInfoCircle
(remove className="text-gray-400") and use the same theme-aware pattern as other
icons by passing the color via the inline prop (e.g.,
color="var(--mantine-color-primary-filled)") so the icon follows CSS variables
and theme changes; update the IconInfoCircle element to use that color prop and
remove the static utility class.
1 participant
Summary by CodeRabbit
New Features
UI/Style Updates