Conversation
Contributor
📝 WalkthroughWalkthroughThe PR adjusts Waves UI styling and layout: replaces rounded corners with top/bottom borders in several components, removes host/grid selection and credits UI from Waves pages and form, simplifies textarea and counter behavior, and adds 16:9 YouTube embed styling in SCSS. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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}
Contributor
There was a problem hiding this comment.
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 (1)
apps/web/src/app/waves/_page.tsx (1)
69-74:⚠️ Potential issue | 🟡 MinorAvoid inline fallback copy in tab labels.
defaultValueintroduces new UI strings in code; these should come from locale resources only.As per coding guidelines,
apps/web/**/*.{ts,tsx}: All new strings must be added to en-US.json only for internationalization.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/waves/_page.tsx` around lines 69 - 74, Remove the inline fallback copy by deleting the defaultValue options from the i18next.t calls for the tab labels (the two places using i18next.t("waves.feed.for-you", ...) and i18next.t("waves.feed.following", ...)), then add the corresponding English strings to the en-US.json locale file under the keys "waves.feed.for-you" and "waves.feed.following" and ensure the label properties continue to call i18next.t with those keys (no defaultValue).
🧹 Nitpick comments (1)
apps/web/src/features/waves/styles/thread-render.scss (1)
130-136: Consider narrowing the iframe selector to the emitted YouTube class.Given
packages/render-helper/src/methods/a.method.ts(Lines 602-614) emitsiframe.youtube-player, targeting that class instead of alliframedescendants makes this safer for future nested iframe variants.♻️ Optional selector tightening
- iframe { + .er-youtube-frame > iframe.youtube-player { position: relative; width: 100%; height: 100%; aspect-ratio: 16 / 9; border-radius: 1rem; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/features/waves/styles/thread-render.scss` around lines 130 - 136, The iframe rule is too broad—change the selector in thread-render.scss from plain iframe to iframe.youtube-player (the class emitted by the renderer) so only the YouTube embeds get these styles; update the rule that sets position, width, height, aspect-ratio and border-radius to target iframe.youtube-player and leave any other iframe styling untouched.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/app/waves/_page.tsx`:
- Around line 89-99: The button currently sets aria-disabled={isDisabled} while
still handling clicks (onClick -> handleFeedTypeChange), which creates
conflicting accessibility semantics; remove or avoid setting aria-disabled when
the tab is intended to be interactive (keep aria-pressed/isActive logic), and
instead convey the login requirement via an explicit accessible hint (e.g., add
an aria-label or aria-describedby referencing a message/tooltip, or set a title)
when the user is unauthenticated; only use aria-disabled on elements that are
truly non-interactive and update references to
isDisabled/isActive/handleFeedTypeChange accordingly.
In `@apps/web/src/features/waves/components/wave-form/wave-form-control.tsx`:
- Line 58: The textarea in the WaveFormControl component removed the browser
focus outline ("outline-none" in the class string) but didn't add an accessible
focus-visible replacement; update the class list for the textarea element in
wave-form-control.tsx (the textarea rendering in the WaveFormControl component)
to include visible keyboard focus styles such as "focus-visible:ring-2
focus-visible:ring-blue-500 focus-visible:ring-offset-1
dark:focus-visible:ring-blue-400" (and keep outline-none if you want to suppress
the default), optionally adding "ring-offset-background" or similar to match
theme; ensure the exact same textarea class string shown in the diff is updated
so keyboard users get a clear focus ring.
---
Outside diff comments:
In `@apps/web/src/app/waves/_page.tsx`:
- Around line 69-74: Remove the inline fallback copy by deleting the
defaultValue options from the i18next.t calls for the tab labels (the two places
using i18next.t("waves.feed.for-you", ...) and i18next.t("waves.feed.following",
...)), then add the corresponding English strings to the en-US.json locale file
under the keys "waves.feed.for-you" and "waves.feed.following" and ensure the
label properties continue to call i18next.t with those keys (no defaultValue).
---
Nitpick comments:
In `@apps/web/src/features/waves/styles/thread-render.scss`:
- Around line 130-136: The iframe rule is too broad—change the selector in
thread-render.scss from plain iframe to iframe.youtube-player (the class emitted
by the renderer) so only the YouTube embeds get these styles; update the rule
that sets position, width, height, aspect-ratio and border-radius to target
iframe.youtube-player and leave any other iframe styling untouched.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4699a957-fafa-4927-a015-40de7cc1379b
📒 Files selected for processing (7)
apps/web/src/app/waves/_components/waves-create-card.tsxapps/web/src/app/waves/_components/waves-list-item.tsxapps/web/src/app/waves/_page.tsxapps/web/src/features/waves/components/wave-form/index.tsxapps/web/src/features/waves/components/wave-form/wave-form-control.tsxapps/web/src/features/waves/components/wave-form/wave-form-toolbar.tsxapps/web/src/features/waves/styles/thread-render.scss
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/src/features/waves/components/wave-form/wave-form-control.tsx (1)
67-71: Optionally keep counter space reserved to prevent small layout jump.Mounting/unmounting the counter can cause a minor vertical shift on first keystroke. Consider always rendering the container and toggling visibility/content only.
Proposed tweak
- {showCounter && ( - <div className={counterClassName} aria-live="polite"> - {textLength}/{characterLimit} - </div> - )} + <div className={clsx(counterClassName, "min-h-4")} aria-live="polite"> + {showCounter ? `${textLength}/${characterLimit}` : <span className="invisible">0/{characterLimit}</span>} + </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/features/waves/components/wave-form/wave-form-control.tsx` around lines 67 - 71, Always render the counter container to reserve layout space instead of conditionally mounting it: replace the conditional JSX around the div that uses showCounter/counterClassName/textLength/characterLimit with an always-mounted <div> using counterClassName and toggle its visible content and accessibility attributes based on showCounter (e.g., render the text "{textLength}/{characterLimit}" when showCounter is true, otherwise render a placeholder like a non‑breaking space or keep the same height via CSS), and set aria-hidden when hidden while keeping aria-live="polite" only when visible to preserve announcement behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/web/src/features/waves/components/wave-form/wave-form-control.tsx`:
- Around line 67-71: Always render the counter container to reserve layout space
instead of conditionally mounting it: replace the conditional JSX around the div
that uses showCounter/counterClassName/textLength/characterLimit with an
always-mounted <div> using counterClassName and toggle its visible content and
accessibility attributes based on showCounter (e.g., render the text
"{textLength}/{characterLimit}" when showCounter is true, otherwise render a
placeholder like a non‑breaking space or keep the same height via CSS), and set
aria-hidden when hidden while keeping aria-live="polite" only when visible to
preserve announcement behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bfca5937-0fd3-4ddd-adc0-7c381ef18e80
📒 Files selected for processing (4)
apps/web/src/app/waves/_page.tsxapps/web/src/features/i18n/locales/en-US.jsonapps/web/src/features/waves/components/wave-form/wave-form-control.tsxapps/web/src/features/waves/styles/thread-render.scss
✅ Files skipped from review due to trivial changes (1)
- apps/web/src/features/i18n/locales/en-US.json
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/src/features/waves/styles/thread-render.scss
- apps/web/src/app/waves/_page.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Refactor
New Features
Style