[render preview] Fix/UI text overflow#669
Conversation
|
@segundavid-dev is attempting to deploy a commit to the QCX-MAIN Team on Vercel. A member of the Team first needs to authorize it. |
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent review details
|
| Layer / File(s) | Summary |
|---|---|
Glassmorphic CSS moved to app layout app/layout.tsx, components/search-results-image.tsx |
glassmorphic/glassmorphic.css is imported in app/layout.tsx and removed from search-results-image.tsx. |
Overflow and truncation utilities across components components/calendar-notepad.tsx, components/chat-panel.tsx, components/search-results-image.tsx, components/video-search-results.tsx, components/empty-screen.tsx |
Adds overflow-hidden, truncate, and line-clamp-2 to multiple UI elements, updates the attachment filename width in chat-panel.tsx, and changes empty-screen button heading rendering to a truncating span. |
Small-mobile overflow Playwright tests tests/responsive.spec.ts |
Adds two responsive tests that verify long attachment filenames and long query text do not overflow their containers or cause horizontal scrolling. |
Estimated code review effort
🎯 2 (Simple) | ⏱️ ~15 minutes
Possibly related PRs
- QueueLab/QCX#422: Also changes the selected-file UI block in
components/chat-panel.tsx. - QueueLab/QCX#473: Also updates truncation-related styling in
components/chat-panel.tsxand similar text-wrapping UI behavior. - QueueLab/QCX#205: Also touches
components/chat-panel.tsxclassName logic for the input area.
Suggested labels
Review effort 1/5
🐇 I hopped through CSS, soft and light,
Clamping long words so screens stay right.
One layout to wear the style anew,
Small-mobile tests now guard the view!
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title clearly matches the PR’s main goal: fixing UI text overflow across multiple components. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
Warning
There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.
🔧 ESLint
If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration.
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 help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/responsive.spec.ts`:
- Around line 226-261: Both tests conditionally skip their validation logic
based on element visibility, which allows them to pass without actually testing
anything. In the test function titled "should not overflow with long attached
file name", add explicit setup to ensure the attachment UI is triggered and
visible before performing the width checks, then use an expect assertion for
visibility instead of the conditional if guard. Similarly, in the test function
titled "should not cause horizontal scroll with long query in search dialog",
add explicit setup to open the search dialog and display the query text before
checking its dimensions, and replace the conditional visibility check with a
direct assertion that the element must be visible. This makes both tests
deterministic by ensuring the responsive overflow checks always execute and
validate the expected UI constraints.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 9a9d4ab4-e605-4ed9-ac78-06d84ddeefe3
📒 Files selected for processing (6)
app/layout.tsxcomponents/calendar-notepad.tsxcomponents/chat-panel.tsxcomponents/search-results-image.tsxcomponents/video-search-results.tsxtests/responsive.spec.ts
📜 Review details
🔇 Additional comments (5)
app/layout.tsx (1)
5-5: LGTM!components/search-results-image.tsx (1)
100-102: LGTM!components/calendar-notepad.tsx (1)
95-95: LGTM!Also applies to: 154-157
components/chat-panel.tsx (1)
244-244: LGTM!Also applies to: 293-301
components/video-search-results.tsx (1)
121-121: LGTM!
| test('should not overflow with long attached file name', async ({ page }) => { | ||
| const attachmentButton = page.locator('[data-testid="mobile-attachment-button"]'); | ||
| if (await attachmentButton.isVisible()) { | ||
| await page.locator('input[type="file"]').setInputFiles({ | ||
| name: 'this-is-a-very-long-filename-that-should-not-overflow-the-container-on-small-screens.pdf', | ||
| mimeType: 'application/pdf', | ||
| buffer: Buffer.from('test') | ||
| }); | ||
|
|
||
| const fileContainer = page.locator('[data-testid="clear-attachment-button"]').locator('..'); | ||
| if (await fileContainer.isVisible()) { | ||
| const containerBox = await fileContainer.boundingBox(); | ||
| expect(containerBox).toBeTruthy(); | ||
| if (containerBox) { | ||
| expect(containerBox.width).toBeLessThanOrEqual(320); | ||
| } | ||
|
|
||
| const bodyWidth = await page.evaluate(() => document.body.scrollWidth); | ||
| expect(bodyWidth).toBeLessThanOrEqual(321); | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| test('should not cause horizontal scroll with long query in search dialog', async ({ page }) => { | ||
| const dialogDesc = page.locator('[role="dialog"] p.line-clamp-2'); | ||
| if (await dialogDesc.isVisible()) { | ||
| const descBox = await dialogDesc.boundingBox(); | ||
| expect(descBox).toBeTruthy(); | ||
| if (descBox) { | ||
| expect(descBox.width).toBeLessThanOrEqual(320); | ||
| } | ||
|
|
||
| const bodyWidth = await page.evaluate(() => document.body.scrollWidth); | ||
| expect(bodyWidth).toBeLessThanOrEqual(321); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make the new small-mobile overflow tests deterministic instead of conditionally skipping.
At Line 228 and Line 251, visibility guards allow both tests to pass without validating anything when elements are absent/closed. Add explicit setup (open dialog/select item, trigger attachment UI) and assert visibility unconditionally before overflow checks.
Suggested tightening
test('should not overflow with long attached file name', async ({ page }) => {
- const attachmentButton = page.locator('[data-testid="mobile-attachment-button"]');
- if (await attachmentButton.isVisible()) {
- await page.locator('input[type="file"]').setInputFiles({
- name: 'this-is-a-very-long-filename-that-should-not-overflow-the-container-on-small-screens.pdf',
- mimeType: 'application/pdf',
- buffer: Buffer.from('test')
- });
+ const attachmentButton = page.locator('[data-testid="mobile-attachment-button"]');
+ await expect(attachmentButton).toBeVisible();
+ await page.locator('input[type="file"]').setInputFiles({
+ name: 'this-is-a-very-long-filename-that-should-not-overflow-the-container-on-small-screens.pdf',
+ mimeType: 'application/pdf',
+ buffer: Buffer.from('test')
+ });
- const fileContainer = page.locator('[data-testid="clear-attachment-button"]').locator('..');
- if (await fileContainer.isVisible()) {
- const containerBox = await fileContainer.boundingBox();
- expect(containerBox).toBeTruthy();
- if (containerBox) {
- expect(containerBox.width).toBeLessThanOrEqual(320);
- }
-
- const bodyWidth = await page.evaluate(() => document.body.scrollWidth);
- expect(bodyWidth).toBeLessThanOrEqual(321);
- }
- }
+ const fileContainer = page.locator('[data-testid="clear-attachment-button"]').locator('..');
+ await expect(fileContainer).toBeVisible();
+ const containerBox = await fileContainer.boundingBox();
+ expect(containerBox).toBeTruthy();
+ if (containerBox) expect(containerBox.width).toBeLessThanOrEqual(320);
+ const bodyWidth = await page.evaluate(() => document.body.scrollWidth);
+ expect(bodyWidth).toBeLessThanOrEqual(321);
});
test('should not cause horizontal scroll with long query in search dialog', async ({ page }) => {
+ // open the search dialog in this test before querying description
+ // (use the existing trigger selector used elsewhere in this suite)
const dialogDesc = page.locator('[role="dialog"] p.line-clamp-2');
- if (await dialogDesc.isVisible()) {
- const descBox = await dialogDesc.boundingBox();
- expect(descBox).toBeTruthy();
- if (descBox) {
- expect(descBox.width).toBeLessThanOrEqual(320);
- }
-
- const bodyWidth = await page.evaluate(() => document.body.scrollWidth);
- expect(bodyWidth).toBeLessThanOrEqual(321);
- }
+ await expect(dialogDesc).toBeVisible();
+ const descBox = await dialogDesc.boundingBox();
+ expect(descBox).toBeTruthy();
+ if (descBox) expect(descBox.width).toBeLessThanOrEqual(320);
+ const bodyWidth = await page.evaluate(() => document.body.scrollWidth);
+ expect(bodyWidth).toBeLessThanOrEqual(321);
});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/responsive.spec.ts` around lines 226 - 261, Both tests conditionally
skip their validation logic based on element visibility, which allows them to
pass without actually testing anything. In the test function titled "should not
overflow with long attached file name", add explicit setup to ensure the
attachment UI is triggered and visible before performing the width checks, then
use an expect assertion for visibility instead of the conditional if guard.
Similarly, in the test function titled "should not cause horizontal scroll with
long query in search dialog", add explicit setup to open the search dialog and
display the query text before checking its dimensions, and replace the
conditional visibility check with a direct assertion that the element must be
visible. This makes both tests deterministic by ensuring the responsive overflow
checks always execute and validate the expected UI constraints.
ngoiyaeric
changed the title
|
render-preview |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
On it |
Summary
PR addressing issue #567
@coderabbitai help confirm if the changes made here correlates with the highlighted changes my you on issue #567
Summary by CodeRabbit
Style
Tests