fix: make horizontal scroll bar thicker on tables#41370
Conversation
WalkthroughUpdated TableWidgetV2 scrollbar sizing: increased exported Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![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
🧹 Nitpick comments (1)
app/client/src/widgets/TableWidgetV2/component/TableStyledWrappers.tsx (1)
68-78: Review the scrollbar sizing logic for consistency and usability.The current implementation has the scrollbar thumb (14px) larger than its track (9px) in the normal state, then shrinking to 10px on hover while the track expands to 12px. This differs from the vertical scrollbar pattern (lines 81-91) where the thumb grows slightly on hover (10px → 11px).
Consider:
- Typically, the thumb should fit within the track dimensions
- Shrinking the thumb on hover (14px → 10px) contradicts the goal of improving visibility and usability
- Align with the vertical scrollbar's hover behavior for consistency
height: ${TABLE_SCROLLBAR_HEIGHT}px; .simplebar-scrollbar { - height: 14px; + height: ${TABLE_SCROLLBAR_HEIGHT}px; } &.simplebar-hover { - height: 12px; + height: 10px; & .simplebar-scrollbar { - height: 10px; + height: 11px; } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/client/src/widgets/TableWidgetV2/component/Constants.ts(1 hunks)app/client/src/widgets/TableWidgetV2/component/TableStyledWrappers.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-10-08T15:32:34.115Z
Learnt from: alex-golovanov
Repo: appsmithorg/appsmith PR: 36665
File: app/client/src/IDE/Components/FileTab/styles.tsx:19-19
Timestamp: 2024-10-08T15:32:34.115Z
Learning: Do not suggest replacing hardcoded `padding-top: 6px;` with design variables in `app/client/src/IDE/Components/FileTab/styles.tsx`.
Applied to files:
app/client/src/widgets/TableWidgetV2/component/Constants.ts
🧬 Code graph analysis (1)
app/client/src/widgets/TableWidgetV2/component/TableStyledWrappers.tsx (1)
app/client/src/widgets/TableWidgetV2/component/Constants.ts (1)
TABLE_SCROLLBAR_HEIGHT(602-602)
⏰ 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). (5)
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-build / client-build
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-prettier / prettier-check
🔇 Additional comments (1)
app/client/src/widgets/TableWidgetV2/component/Constants.ts (1)
602-602: Confirm scrollbar thickness increase adequately addresses user visibility concerns.GitHub issue #37549 documents that users struggle to identify the horizontal scrollbar as it's too thin, with some unaware the table is even scrollable. The 1px increase (8px → 9px) is modest relative to the problem severity. At 9px, the scrollbar remains substantially thinner than standard browser scrollbars (15-17px).
Verify this increase meets design specifications and user accessibility needs, or consider a more substantial increase.
app/client/src/widgets/TableWidgetV2/component/TableStyledWrappers.tsx
Outdated
Show resolved
Hide resolved
|
/build-deploy-preview |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/19321214367. |
|
Deploy-Preview-URL: https://ce-41370.dp.appsmith.com |
|
/build-deploy-preview |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/19323676531. |
|
Deploy-Preview-URL: https://ce-41370.dp.appsmith.com |
… for hidden horizontal scrollbar
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/scrollbar_spec.ts (1)
21-21: Consider clarifying the constant duplication.The hardcoded constant creates a maintenance risk if
TABLE_SCROLLBAR_HEIGHTchanges inConstants.ts. While direct imports from application source may not be feasible in Cypress tests, consider:
- Updating the comment to explicitly state this is a duplicate that must be kept in sync
- Or exploring if the constant can be imported via a shared test utilities module
Example comment improvement:
- const TABLE_SCROLLBAR_HEIGHT = 10; // From Constants.ts + // Must match TABLE_SCROLLBAR_HEIGHT in Constants.ts - duplicated because Cypress tests can't import app source directly + const TABLE_SCROLLBAR_HEIGHT = 10;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/scrollbar_spec.ts(1 hunks)app/client/src/widgets/TableWidgetV2/component/Constants.ts(1 hunks)app/client/src/widgets/TableWidgetV2/component/TableStyledWrappers.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/client/src/widgets/TableWidgetV2/component/TableStyledWrappers.tsx
🧰 Additional context used
📓 Path-based instructions (1)
app/client/cypress/**/**.*
⚙️ CodeRabbit configuration file
app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
Files:
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/scrollbar_spec.ts
🧠 Learnings (4)
📚 Learning: 2024-11-13T09:11:36.959Z
Learnt from: vhemery
Repo: appsmithorg/appsmith PR: 37371
File: app/client/cypress/e2e/Regression/ClientSide/Widgets/Image/Image_base64_spec.js:35-37
Timestamp: 2024-11-13T09:11:36.959Z
Learning: In the file `app/client/cypress/e2e/Regression/ClientSide/Widgets/Image/Image_base64_spec.js`, when writing Cypress end-to-end tests for the Image Widget in JavaScript, use `viewWidgetsPage.imageinner` as the selector when asserting 'src' and 'alt' attributes to maintain consistency.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/scrollbar_spec.ts
📚 Learning: 2025-11-05T12:56:49.249Z
Learnt from: vsvamsi1
Repo: appsmithorg/appsmith PR: 41363
File: app/client/cypress/e2e/Regression/ClientSide/StaticUrl/PagePersistance_Spec.ts:0-0
Timestamp: 2025-11-05T12:56:49.249Z
Learning: In the file `app/client/cypress/e2e/Regression/ClientSide/StaticUrl/PagePersistance_Spec.ts`, the test suite intentionally uses sequential tests where Tests 4-6 depend on state (`page1Slug`, `page2Slug`) set in Tests 2-3. The author (vsvamsi1) prioritizes readability by breaking the flow into different segments over strict test isolation.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/scrollbar_spec.ts
📚 Learning: 2025-11-05T12:58:32.486Z
Learnt from: vsvamsi1
Repo: appsmithorg/appsmith PR: 41363
File: app/client/cypress/e2e/Regression/ClientSide/StaticUrl/SlugPersistance_Spec.ts:32-225
Timestamp: 2025-11-05T12:58:32.486Z
Learning: In the file `app/client/cypress/e2e/Regression/ClientSide/StaticUrl/SlugPersistance_Spec.ts`, the test suite intentionally uses sequential tests where Tests 2-5 depend on state from previous tests (e.g., slug values modified in earlier tests). The author (vsvamsi1) prioritizes readability by breaking the flow into different segments over strict test isolation.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/scrollbar_spec.ts
📚 Learning: 2024-10-08T15:32:34.115Z
Learnt from: alex-golovanov
Repo: appsmithorg/appsmith PR: 36665
File: app/client/src/IDE/Components/FileTab/styles.tsx:19-19
Timestamp: 2024-10-08T15:32:34.115Z
Learning: Do not suggest replacing hardcoded `padding-top: 6px;` with design variables in `app/client/src/IDE/Components/FileTab/styles.tsx`.
Applied to files:
app/client/src/widgets/TableWidgetV2/component/Constants.ts
🧬 Code graph analysis (1)
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/scrollbar_spec.ts (1)
app/client/src/widgets/TableWidgetV2/component/Constants.ts (1)
TABLE_SCROLLBAR_HEIGHT(602-602)
⏰ 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). (8)
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: perform-test / rts-build / build
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-build / client-build
🔇 Additional comments (1)
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/scrollbar_spec.ts (1)
28-36: LGTM! Test logic correctly accounts for scrollbar height.The assertion properly accounts for the implementation detail that the scroll container always subtracts
TABLE_SCROLLBAR_HEIGHTfrom the table height. The explanatory comment adds valuable context.
|
|
||
| export const TABLE_SCROLLBAR_WIDTH = 10; | ||
| export const TABLE_SCROLLBAR_HEIGHT = 8; | ||
| export const TABLE_SCROLLBAR_HEIGHT = 10; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify all usages of TABLE_SCROLLBAR_HEIGHT have been updated.
The constant increase from 8 to 10 aligns with the PR objective. Ensure all dependent code accounts for this change, particularly any hardcoded calculations or CSS that might reference the old value.
Run the following script to identify any hardcoded references to the old value or other usages:
🏁 Script executed:
#!/bin/bash
# Search for hardcoded scrollbar height values that might need updating
rg -n --type ts --type tsx --type css -C3 '\b8px\b|\bheight:\s*8\b' app/client/src/widgets/TableWidgetV2/
# Search for all usages of TABLE_SCROLLBAR_HEIGHT to verify they handle the new value
rg -n --type ts --type tsx -C3 'TABLE_SCROLLBAR_HEIGHT' app/client/src/widgets/TableWidgetV2/Length of output: 55
🏁 Script executed:
#!/bin/bash
# Find all ts/tsx files and search for hardcoded scrollbar heights
fd -e ts -e tsx 'TableWidgetV2' app/client/src/widgets/ | xargs rg -n '\b8px\b|\bheight:\s*8\b' -C3
# Search for all usages of TABLE_SCROLLBAR_HEIGHT across the entire codebase
rg -n 'TABLE_SCROLLBAR_HEIGHT' app/client/src/ -C2
# Also check if old value (8) appears in test files or specs
fd -e ts -e tsx -e spec 'test|spec' app/client/src/widgets/TableWidgetV2/ | xargs rg -n '\b8\b' -C2Length of output: 263785
Update hardcoded scrollbar heights in TableStyledWrappers.tsx to match constant change.
The constant increase from 8 to 10 is incomplete. In app/client/src/widgets/TableWidgetV2/component/TableStyledWrappers.tsx, lines 71 and 76 contain hardcoded height: 8px; inside .simplebar-scrollbar selectors that should be updated to 10px to maintain visual consistency with the container height at line 68.
🤖 Prompt for AI Agents
In app/client/src/widgets/TableWidgetV2/component/TableStyledWrappers.tsx around
lines 68–76, update the two hardcoded scrollbar heights (currently "height:
8px;" at lines 71 and 76 inside the .simplebar-scrollbar selectors) to "height:
10px;" so they match the TABLE_SCROLLBAR_HEIGHT constant change and the
container height defined at line 68; verify both selectors are updated to
maintain visual consistency.
rahulbarwal
deleted the
fix/issue-37549/horizontal-scroll-height-table
branch
2 participants
Description
Tip
Add a TL;DR when the description is longer than 500 words or extremely technical (helps the content, marketing, and DevRel team).
Please also include relevant motivation and context. List any dependencies that are required for this change. Add links to Notion, Figma or any other documents that might be relevant to the PR.
Fixes #
37549Warning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags="@tag.Table"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/19530806854
Commit: d4cdfb8
Cypress dashboard.
Tags:
@tag.TableSpec:
Thu, 20 Nov 2025 09:21:32 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.