fix: Reverting the previous fix for table scrollbar and hiding vertical scrollbar when needed #33806

ankitakinger · 2024-05-28T13:33:20Z

Description

Reverting the previous fix for table scrollbar and hiding vertical scrollbar when needed, with a new fix.

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/9287519606
Commit: f4d3b4e
Cypress dashboard url: Click here!

Communication

Should the DevRel and Marketing teams inform users about this change?

Yes
No

…hen needed

coderabbitai · 2024-05-28T13:33:28Z

Walkthrough

The changes primarily focus on improving the TableWidgetV2 component by refining the height and page size calculations, and simplifying item size calculations. Additionally, test assertions in Cypress tests have been updated to reflect these changes. These modifications aim to address issues related to table rendering, particularly when cell wrapping is enabled.

Changes

File Path	Change Summary
`.../TableWidgetV2/component/Table.tsx`	Adjusted `scrollContainerStyles` height calculation by removing conditional checks based on data length and page size.
`.../TableWidgetV2/widget/derived.js`	Updated `getPageSize` logic to include a check based on `props.tableData.length`.
`.../TableWidgetV2/component/TableBody/index.tsx`	Simplified `itemSize` calculation by removing `ROW_VIRTUAL_OFFSET`.
`.../TableWidgetV2/Pagesize_spec.ts`	Adjusted expected text values in assertions by decreasing them by 1.
`.../TableWidgetV2/scrollbar_spec.ts`	Removed addition of `2` to client height comparison in a test assertion.

Assessment against linked issues

Objective	Addressed	Explanation
Fix issue with vertical scrollbar missing when cell wrapping is enabled (#33774)	✅

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai generate interesting stats about this repository and render them as a table.
- @coderabbitai show all the console.log statements in this repository.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

ankitakinger · 2024-05-28T13:33:44Z

/build-deploy-preview skip-tests=true

github-actions · 2024-05-28T13:34:06Z

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/9270350713.
Workflow: On demand build Docker image and deploy preview.
skip-tests: true.
env: ``.
PR: 33806.
recreate: .

coderabbitai

Actionable comments posted: 0

Outside diff range and nitpick comments (7)

app/client/src/widgets/TableWidgetV2/component/Table.tsx (5)
Line range hint 62-62: Consider replacing the template literal with a regular string since no interpolation is used.
- border-radius: ${({ borderRadius }) =>
-   borderRadius >= `1.5rem` ? `0.375rem` : borderRadius} !important;
+ border-radius: ${({ borderRadius }) =>
+   borderRadius >= '1.5rem' ? '0.375rem' : borderRadius} !important;
Line range hint 104-104: Specify explicit types instead of using any.

Using any type defeats the purpose of TypeScript's static typing. It's better to specify explicit types for better type checking and to avoid potential runtime errors.

Also applies to: 152-152, 162-162, 168-168

Line range hint 246-249: Convert this function expression to an arrow function for consistency and better handling of this context.
- setTimeout(function () {
+ setTimeout(() => {
Line range hint 285-285: Remove redundant Boolean conversion.
- toggleAllRowSelect(!Boolean(rowSelectionState), page);
+ toggleAllRowSelect(!rowSelectionState, page);
Line range hint 321-321: Use optional chaining to safely access nested properties.
- if (scrollBarRef && scrollBarRef?.current) {
+ if (scrollBarRef?.current) {
app/client/src/widgets/TableWidgetV2/widget/derived.js (2)
Line range hint 56-58: Consider using for...of instead of forEach for better performance and readability.

Using for...of allows for easier control flow management (like break or continue) and can be more performant in some cases.

Also applies to: 105-107, 229-241, 249-257, 268-272, 283-313, 690-709

Line range hint 246-246: Use optional chaining to simplify property access and prevent runtime errors.
- if (column && column.displayText) {
+ if (column?.displayText) {
Also applies to: 269-269, 286-286, 328-328, 618-618

github-actions · 2024-05-28T13:47:57Z

Deploy-Preview-URL: https://ce-33806.dp.appsmith.com

coderabbitai

Actionable comments posted: 2

Outside diff range and nitpick comments (2)

app/client/src/widgets/TableWidgetV2/component/TableBody/index.tsx (2)
Line range hint 113-113: Avoid using the index as a key in React lists. Use a unique identifier from the row data if available.
- key={index}
+ key={row.id} // Assuming 'id' is a unique identifier in the row data
Line range hint 67-69: The else clause here is redundant since the previous branches break early. Consider simplifying the logic.
- } else {
-   return <EmptyRow style={style} />;
- }
+ return <EmptyRow style={style} />;

app/client/src/widgets/TableWidgetV2/component/TableBody/index.tsx

coderabbitai

Actionable comments posted: 0

Outside diff range and nitpick comments (2)

app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/scrollbar_spec.ts (1)
Line range hint 19-30: Convert the function expression to an arrow function for consistency with modern JavaScript practices.
- before(() => {
+ before(() => {
app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/Pagesize_spec.ts (1)
Line range hint 9-46: Convert the function expression to an arrow function to align with modern JavaScript practices.
- it("1. should test that pageSize is computed properly for all the row sizes", function () {
+ it("1. should test that pageSize is computed properly for all the row sizes", () => {

coderabbitai

Actionable comments posted: 0

Outside diff range and nitpick comments (1)

app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/Pagesize_spec.ts (1)
Line range hint 9-46: Consider refactoring this function expression into an arrow function for consistency and improved readability.
- function () {
+ () => {

ankitakinger · 2024-05-29T07:29:50Z

/build-deploy-preview recreate=true skip-tests=true

github-actions · 2024-05-29T07:30:12Z

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/9282022129.
Workflow: On demand build Docker image and deploy preview.
skip-tests: true.
env: ``.
PR: 33806.
recreate: true.

Want to double-check

github-actions · 2024-05-29T07:42:47Z

Deploy-Preview-URL: https://ce-33806.dp.appsmith.com

ankitakinger · 2024-05-29T09:41:57Z

/build-deploy-preview recreate=true skip-tests=true

github-actions · 2024-05-29T09:42:19Z

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/9283849994.
Workflow: On demand build Docker image and deploy preview.
skip-tests: true.
env: ``.
PR: 33806.
recreate: true.

github-actions · 2024-05-29T09:57:12Z

Deploy-Preview-URL: https://ce-33806.dp.appsmith.com

ankitakinger · 2024-05-29T12:05:20Z

/build-deploy-preview recreate=true skip-tests=true

github-actions · 2024-05-29T12:05:40Z

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/9285735821.
Workflow: On demand build Docker image and deploy preview.
skip-tests: true.
env: ``.
PR: 33806.
recreate: true.

github-actions · 2024-05-29T12:19:28Z

Deploy-Preview-URL: https://ce-33806.dp.appsmith.com

coderabbitai

Actionable comments posted: 0

Outside diff range and nitpick comments (1)

app/client/cypress/e2e/Regression/ClientSide/Widgets/TableV2/Pagesize_spec.ts (1)
Line range hint 9-46: Refactor to use arrow functions for consistency and improved readability.
- function () {
+ () => {
Apply this change throughout the test suite where traditional function expressions are used.

Also applies to: 8-47

…o fix/table-scrollbar-visibility

ankitakinger · 2024-05-29T13:14:27Z

/build-deploy-preview recreate=true skip-tests=true

github-actions · 2024-05-29T13:14:48Z

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/9286802499.
Workflow: On demand build Docker image and deploy preview.
skip-tests: true.
env: ``.
PR: 33806.
recreate: true.

github-actions · 2024-05-29T13:27:53Z

Deploy-Preview-URL: https://ce-33806.dp.appsmith.com

pratapaprasanna · 2024-05-29T13:46:37Z

/build-deploy-preview skip-tests=true

github-actions · 2024-05-29T13:46:58Z

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/9287305930.
Workflow: On demand build Docker image and deploy preview.
skip-tests: true.
env: ``.
PR: 33806.
recreate: .

github-actions · 2024-05-29T13:59:43Z

Deploy-Preview-URL: https://ce-33806.dp.appsmith.com

pratapaprasanna · 2024-05-29T14:34:46Z

/build-deploy-preview skip-tests=true

github-actions · 2024-05-29T14:35:06Z

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/9288088611.
Workflow: On demand build Docker image and deploy preview.
skip-tests: true.
env: ``.
PR: 33806.
recreate: .

github-actions · 2024-05-29T14:47:54Z

Deploy-Preview-URL: https://ce-33806.dp.appsmith.com

kamakshibhat-appsmith · 2024-05-30T09:20:02Z

Looks good,
Only problem I could see is - there is no existing way to remove scrollbar from Cell wrapping enabled table. Will create a separate issue to track that part
@ankitakinger

reverting the previous fix for table scrollbar and hiding scrollbar w…

b96d0c0

…hen needed

ankitakinger added the ok-to-test Required label for CI label May 28, 2024

coderabbitai bot reviewed May 28, 2024

View reviewed changes

fixing scrollbar for virtual table

a4b088c

coderabbitai bot reviewed May 28, 2024

View reviewed changes

app/client/src/widgets/TableWidgetV2/component/TableBody/index.tsx Show resolved Hide resolved

ankitakinger added 2 commits May 28, 2024 20:28

fixing scrollbar spec

3de294f

fixing pagesize spec

2b438fc

ankitakinger added ok-to-test Required label for CI and removed ok-to-test Required label for CI labels May 28, 2024

coderabbitai bot reviewed May 28, 2024

View reviewed changes

ankitakinger added 2 commits May 28, 2024 21:30

fixing pagesize spec

6bcaf11

fixing pagesize spec

36a7a31

ankitakinger added ok-to-test Required label for CI and removed ok-to-test Required label for CI labels May 28, 2024

coderabbitai bot reviewed May 28, 2024

View reviewed changes

ankitakinger requested a review from jsartisan May 29, 2024 07:29

jsartisan previously approved these changes May 29, 2024

View reviewed changes

fiixng failing integration test

baa5064

ankitakinger added ok-to-test Required label for CI and removed ok-to-test Required label for CI labels May 29, 2024

coderabbitai bot reviewed May 29, 2024

View reviewed changes

Merge branch 'release' of https://github.com/appsmithorg/appsmith int…

f4d3b4e

…o fix/table-scrollbar-visibility

ankitakinger added ok-to-test Required label for CI and removed ok-to-test Required label for CI labels May 29, 2024

ankitakinger requested a review from jsartisan May 30, 2024 05:03

jsartisan approved these changes May 30, 2024

View reviewed changes

ankitakinger merged commit 595e7da into release May 30, 2024
47 checks passed

ankitakinger deleted the fix/table-scrollbar-visibility branch May 30, 2024 09:40

kamakshibhat-appsmith mentioned this pull request Jun 10, 2024

[Bug]: There is no existing way to remove scrollbar from Cell wrapping enabled table. #34149

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Reverting the previous fix for table scrollbar and hiding vertical scrollbar when needed #33806

fix: Reverting the previous fix for table scrollbar and hiding vertical scrollbar when needed #33806

ankitakinger commented May 28, 2024 •

edited

Loading

coderabbitai bot commented May 28, 2024 •

edited

Loading

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (`.coderabbit.yaml`)

Documentation and Community

ankitakinger commented May 28, 2024

github-actions bot commented May 28, 2024

coderabbitai bot left a comment

github-actions bot commented May 28, 2024

coderabbitai bot left a comment

coderabbitai bot left a comment

coderabbitai bot left a comment

ankitakinger commented May 29, 2024

github-actions bot commented May 29, 2024

github-actions bot commented May 29, 2024

ankitakinger commented May 29, 2024

github-actions bot commented May 29, 2024

github-actions bot commented May 29, 2024

ankitakinger commented May 29, 2024

github-actions bot commented May 29, 2024

github-actions bot commented May 29, 2024

coderabbitai bot left a comment

ankitakinger commented May 29, 2024

github-actions bot commented May 29, 2024

github-actions bot commented May 29, 2024

pratapaprasanna commented May 29, 2024

github-actions bot commented May 29, 2024

github-actions bot commented May 29, 2024

pratapaprasanna commented May 29, 2024

github-actions bot commented May 29, 2024

github-actions bot commented May 29, 2024

kamakshibhat-appsmith commented May 30, 2024

fix: Reverting the previous fix for table scrollbar and hiding vertical scrollbar when needed #33806

fix: Reverting the previous fix for table scrollbar and hiding vertical scrollbar when needed #33806

Conversation

ankitakinger commented May 28, 2024 • edited Loading

Description

Automation

🔍 Cypress test results

Communication

coderabbitai bot commented May 28, 2024 • edited Loading

Walkthrough

Changes

Assessment against linked issues

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (.coderabbit.yaml)

Documentation and Community

ankitakinger commented May 28, 2024

github-actions bot commented May 28, 2024

coderabbitai bot left a comment

Choose a reason for hiding this comment

github-actions bot commented May 28, 2024

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

ankitakinger commented May 29, 2024

github-actions bot commented May 29, 2024

github-actions bot commented May 29, 2024

ankitakinger commented May 29, 2024

github-actions bot commented May 29, 2024

github-actions bot commented May 29, 2024

ankitakinger commented May 29, 2024

github-actions bot commented May 29, 2024

github-actions bot commented May 29, 2024

coderabbitai bot left a comment

Choose a reason for hiding this comment

ankitakinger commented May 29, 2024

github-actions bot commented May 29, 2024

github-actions bot commented May 29, 2024

pratapaprasanna commented May 29, 2024

github-actions bot commented May 29, 2024

github-actions bot commented May 29, 2024

pratapaprasanna commented May 29, 2024

github-actions bot commented May 29, 2024

github-actions bot commented May 29, 2024

kamakshibhat-appsmith commented May 30, 2024

ankitakinger commented May 28, 2024 •

edited

Loading

coderabbitai bot commented May 28, 2024 •

edited

Loading

CodeRabbit Configration File (`.coderabbit.yaml`)