Skip to content

♿️(frontend) set explicit document title on recording download page#1261

Merged
lebaudantoine merged 1 commit intomainfrom
fix/recording-page-title
Apr 13, 2026
Merged

♿️(frontend) set explicit document title on recording download page#1261
lebaudantoine merged 1 commit intomainfrom
fix/recording-page-title

Conversation

@Ovgodd
Copy link
Copy Markdown
Collaborator

@Ovgodd Ovgodd commented Apr 10, 2026

Purpose

The browser tab showed only the generic app title (e.g. “LaSuite Meet” / “Visio”) on the recording download page, so users could not quickly tell which tab or context they were in.

Capture d'écran 2026-04-10 095829

Proposal

  • Explicit tab title when the recording is ready (success.title)
  • Title aligned with the “not ready” state (unsaved.title) when the recording status is not saved / not downloadable ye

@Ovgodd Ovgodd requested a review from lebaudantoine April 10, 2026 08:01
@Ovgodd Ovgodd self-assigned this Apr 10, 2026
@Ovgodd Ovgodd added the a11y label Apr 10, 2026
@Ovgodd Ovgodd force-pushed the fix/recording-page-title branch from f2721f1 to 3d46f85 Compare April 10, 2026 08:02
@Ovgodd Ovgodd marked this pull request as ready for review April 10, 2026 08:02
@Ovgodd Ovgodd linked an issue Apr 10, 2026 that may be closed by this pull request
Copy link
Copy Markdown
Collaborator

@lebaudantoine lebaudantoine left a comment

Choose a reason for hiding this comment

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

LGTM; I've not tested it locally. Please double check it, to make sure you don't introduce any regression.

@lebaudantoine
Copy link
Copy Markdown
Collaborator

@coderabbitai full review

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 12, 2026

✅ Actions performed

Full review triggered.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 12, 2026

Walkthrough

This pull request adds dynamic document title handling to the RecordingDownload page. The implementation imports the useTitle hook and useMemo, derives a page title based on recording status conditions (isSaved, error state, and expiration status), and applies it to update the browser title. The CHANGELOG.md is updated to document this new feature and correct minor whitespace formatting.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: adding explicit document title handling to the recording download page frontend component.
Description check ✅ Passed The description is directly related to the changeset, explaining the purpose (generic tab title problem) and the implemented proposal (dynamic titles per state).
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a 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

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/frontend/src/features/recording/routes/RecordingDownload.tsx`:
- Around line 63-65: The tab title precedence is inverted: the title checks
evaluate data?.is_expired before isSaved but the UI renders the unsaved state
first; update the title logic in RecordingDownload so it checks the unsaved
condition before the expired condition (i.e., evaluate data && !isSaved ->
`${APP_TITLE} - ${t('unsaved.title')}` prior to checking data?.is_expired),
keeping the existing checks for data && isSaved and the same t(...) keys so the
tab title matches the rendered page state.
- Around line 56-59: The downloadable-status check is duplicated: you already
compute isSaved (using RecordingStatus.Saved,
RecordingStatus.NotificationSucceed, RecordingStatus.FailedToStop) but later
re-check the same set in the render guard; update the render condition inside
the RecordingDownload component to reuse the isSaved boolean instead of
repeating the status array so the single source of truth (isSaved) is used for
both logic and rendering.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8e588060-babe-45ca-865a-2a954036330f

📥 Commits

Reviewing files that changed from the base of the PR and between 170763a and 3d46f85.

📒 Files selected for processing (2)
  • CHANGELOG.md
  • src/frontend/src/features/recording/routes/RecordingDownload.tsx

Comment on lines +56 to +59
const isSaved =
data?.status === RecordingStatus.Saved ||
data?.status === RecordingStatus.NotificationSucceed ||
data?.status === RecordingStatus.FailedToStop
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Reuse isSaved in the render guard to avoid status drift.

isSaved centralizes the downloadable statuses, but the same status set is duplicated again later in the component. Reusing isSaved in the render condition prevents future divergence.

Suggested refactor
-  if (
-    data.status !== RecordingStatus.Saved &&
-    data.status !== RecordingStatus.NotificationSucceed &&
-    data.status !== RecordingStatus.FailedToStop
-  ) {
+  if (!isSaved) {
     return <ErrorScreen title={t('unsaved.title')} body={t('unsaved.body')} />
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/frontend/src/features/recording/routes/RecordingDownload.tsx` around
lines 56 - 59, The downloadable-status check is duplicated: you already compute
isSaved (using RecordingStatus.Saved, RecordingStatus.NotificationSucceed,
RecordingStatus.FailedToStop) but later re-check the same set in the render
guard; update the render condition inside the RecordingDownload component to
reuse the isSaved boolean instead of repeating the status array so the single
source of truth (isSaved) is used for both logic and rendering.

Comment thread src/frontend/src/features/recording/routes/RecordingDownload.tsx Outdated
@Ovgodd Ovgodd force-pushed the fix/recording-page-title branch from 057b76d to 3bc0fd1 Compare April 13, 2026 09:24
RecordingDownload now updates the tab title per state
@lebaudantoine lebaudantoine force-pushed the fix/recording-page-title branch from 3bc0fd1 to f0fda14 Compare April 13, 2026 18:18
@lebaudantoine lebaudantoine merged commit f0fda14 into main Apr 13, 2026
22 checks passed
@sonarqubecloud
Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Page title "Your recording is ready!" incomplete

2 participants