Conversation
WalkthroughThe changes focus on enhancing the functionality and behavior of the WDSTableWidget component. A key addition is the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WDSTableWidget
participant renderMode
participant UI
User->>WDSTableWidget: Open Table
WDSTableWidget->>renderMode: Determine Mode (Canvas / Preview / Deploy)
renderMode-->>WDSTableWidget: Mode
WDSTableWidget->>UI: Update Table Component
Note right of UI: Check disableScroll property
UI-->>User: Render Table with/without Scroll
Poem
Tip AI model upgrade
|
github-actions
bot
added
the
skip-changelog
![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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (6)
app/client/cypress/snapshots/AnvilTableWidgetSnapshot_spec.ts/anvilTableWidgetCanvas.snap.pngis excluded by!**/*.pngapp/client/cypress/snapshots/AnvilTableWidgetSnapshot_spec.ts/anvilTableWidgetCanvasDark.snap.pngis excluded by!**/*.pngapp/client/cypress/snapshots/AnvilTableWidgetSnapshot_spec.ts/anvilTableWidgetDeployipad-2.snap.pngis excluded by!**/*.pngapp/client/cypress/snapshots/AnvilTableWidgetSnapshot_spec.ts/anvilTableWidgetDeployiphone-6.snap.pngis excluded by!**/*.pngapp/client/cypress/snapshots/AnvilTableWidgetSnapshot_spec.ts/anvilTableWidgetDeploymacbook-13.snap.pngis excluded by!**/*.pngapp/client/cypress/snapshots/AnvilTableWidgetSnapshot_spec.ts/anvilTableWidgetPreview.snap.pngis excluded by!**/*.png
Files selected for processing (6)
- app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/AnvilTableWidgetSnapshot_spec.ts (1 hunks)
- app/client/src/widgets/wds/WDSTableWidget/component/Table.tsx (2 hunks)
- app/client/src/widgets/wds/WDSTableWidget/component/index.tsx (4 hunks)
- app/client/src/widgets/wds/WDSTableWidget/component/styles.module.css (2 hunks)
- app/client/src/widgets/wds/WDSTableWidget/widget/index.tsx (1 hunks)
- app/client/src/widgets/wds/WDSTableWidget/widget/reactTableUtils/getColumnsPureFn.tsx (2 hunks)
Files skipped from review due to trivial changes (1)
- app/client/src/widgets/wds/WDSTableWidget/widget/reactTableUtils/getColumnsPureFn.tsx
Additional context used
Path-based instructions (5)
app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/AnvilTableWidgetSnapshot_spec.ts (1)
Pattern
**/*.*: Do not use cy.wait in code.
Do call the add function.
Do not keep duplicate lines in code.
Descriptive test names are used to clearly convey the intent of each test.app/client/src/widgets/wds/WDSTableWidget/component/styles.module.css (1)
Pattern
**/*.*: Do not use cy.wait in code.
Do call the add function.
Do not keep duplicate lines in code.
Descriptive test names are used to clearly convey the intent of each test.app/client/src/widgets/wds/WDSTableWidget/component/index.tsx (1)
Pattern
**/*.*: Do not use cy.wait in code.
Do call the add function.
Do not keep duplicate lines in code.
Descriptive test names are used to clearly convey the intent of each test.app/client/src/widgets/wds/WDSTableWidget/component/Table.tsx (1)
Pattern
**/*.*: Do not use cy.wait in code.
Do call the add function.
Do not keep duplicate lines in code.
Descriptive test names are used to clearly convey the intent of each test.app/client/src/widgets/wds/WDSTableWidget/widget/index.tsx (1)
Pattern
**/*.*: Do not use cy.wait in code.
Do call the add function.
Do not keep duplicate lines in code.
Descriptive test names are used to clearly convey the intent of each test.
Biome
app/client/src/widgets/wds/WDSTableWidget/widget/index.tsx
[error] 930-930: Avoid redundant
BooleancallIt is not necessary to use
Booleancall when a value will already be coerced to a boolean.
Unsafe fix: Remove redundantBooleancall(lint/complexity/noExtraBooleanCast)
Additional comments not posted (9)
app/client/cypress/e2e/Regression/ClientSide/Anvil/Widgets/AnvilTableWidgetSnapshot_spec.ts (1)
1-27: LGTM! The test structure is clear and well-organized.The tests cover different modes of the Anvil Table Widget effectively.
app/client/src/widgets/wds/WDSTableWidget/component/styles.module.css (2)
37-39: LGTM! The[data-disable-scroll]block is correctly implemented.It effectively disables scrolling when the attribute is set.
170-172: LGTM! The styling for the last cell in the row is handled correctly.Setting
border-block-end-colortotransparentis a good design choice.app/client/src/widgets/wds/WDSTableWidget/component/index.tsx (2)
85-85: LGTM! ThedisableScrollproperty is correctly added to theReactTableComponentPropsinterface.This addition is necessary for controlling the scroll behavior.
102-102: LGTM! ThedisableScrollproperty is correctly utilized in theReactTableComponentfunction.Passing this property to the
Tablecomponent ensures the intended behavior.Also applies to: 214-214
app/client/src/widgets/wds/WDSTableWidget/component/Table.tsx (2)
101-101: LGTM! ThedisableScrollproperty is correctly added to theTablePropsinterface.This addition is necessary for controlling the scroll behavior.
298-298: LGTM! ThedisableScrollproperty is correctly utilized in theTablecomponent.Applying this property as a data attribute ensures the intended behavior.
app/client/src/widgets/wds/WDSTableWidget/widget/index.tsx (2)
929-931: LGTM! ThedisableScrollproperty is correctly utilized in theReactTableComponent.Passing this property ensures the intended behavior.
Tools
Biome
[error] 930-930: Avoid redundant
BooleancallIt is not necessary to use
Booleancall when a value will already be coerced to a boolean.
Unsafe fix: Remove redundantBooleancall(lint/complexity/noExtraBooleanCast)
Line range hint
319-319: LGTM! ThedisableScrollproperty is correctly included in the comparison forReact.memo.Ensuring the component re-renders if this property changes is important for correct functionality.
Tools
Biome
[error] 930-930: Avoid redundant
BooleancallIt is not necessary to use
Booleancall when a value will already be coerced to a boolean.
Unsafe fix: Remove redundantBooleancall(lint/complexity/noExtraBooleanCast)
| disableScroll={ | ||
| this.props.renderMode === RenderModes.CANVAS && | ||
| !Boolean(this.props.isPreviewMode) | ||
| } |
There was a problem hiding this comment.
Fix redundant Boolean call.
The Boolean call is redundant since the value is already coerced to a boolean.
- !Boolean(this.props.isPreviewMode)
+ !this.props.isPreviewModeCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| disableScroll={ | |
| this.props.renderMode === RenderModes.CANVAS && | |
| !Boolean(this.props.isPreviewMode) | |
| } | |
| disableScroll={ | |
| this.props.renderMode === RenderModes.CANVAS && | |
| !this.props.isPreviewMode | |
| } |
Tools
Biome
[error] 930-930: Avoid redundant
BooleancallIt is not necessary to use
Booleancall when a value will already be coerced to a boolean.
Unsafe fix: Remove redundantBooleancall(lint/complexity/noExtraBooleanCast)
## Description 1. Add scripts for local e2e testing in the docker container. 2. Add types for cypress-image-snapshot In this PR, I also added the following PRS as they affect screenshot tests as well. I did this to speed up the process and unblock the team. #34528 #34546 #34676 #34729 #34638 #34639 #34511 To run E2E tests locally in docker, you need to do the following: 1. Run FE locally and prepare the tests for local launch. See the instructions [here](https://github.com/appsmithorg/appsmith/blob/release/contributions/ClientSetup.md). 2. Run `yarn cypress:snapshot:docker:build` — this will create a docker container with the necessary environment. 3. Run `yarn cypress:snapshot:docker "./cypress/e2e/Regression/ClientSide/Anvil/Widgets/*_spec.ts" updateSnapshots=false`. Here we can use the path to a specific file, or set `updateSnapshots=true` flag to update the screenshots. ## Automation /ok-to-test tags="@tag.Anvil" ### :mag: Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/9844579277> > Commit: 75f2659 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=9844579277&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Anvil` > Spec: > <hr>Mon, 08 Jul 2024 18:37:36 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added support for image snapshot testing with the `@types/cypress-image-snapshot` dependency. - **Refactor** - Updated test specifications by removing unnecessary `triggerInputInvalidState()` calls. - Reorganized and improved efficiency of image snapshot methods for various devices. - **Chores** - Updated `Dockerfile` to configure the Cypress environment with specific versions for dependencies. - Changed import paths in `e2e.js` for better module resolution. - **Style** - Fixed a comment typo in Cypress plugin configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Pawan Kumar <pawankumar@Pawans-MacBook-Pro-2.local> Co-authored-by: unknown <vadim@appsmith.com>
|
Merged here. |
## Description 1. Add scripts for local e2e testing in the docker container. 2. Add types for cypress-image-snapshot In this PR, I also added the following PRS as they affect screenshot tests as well. I did this to speed up the process and unblock the team. appsmithorg#34528 appsmithorg#34546 appsmithorg#34676 appsmithorg#34729 appsmithorg#34638 appsmithorg#34639 appsmithorg#34511 To run E2E tests locally in docker, you need to do the following: 1. Run FE locally and prepare the tests for local launch. See the instructions [here](https://github.com/appsmithorg/appsmith/blob/release/contributions/ClientSetup.md). 2. Run `yarn cypress:snapshot:docker:build` — this will create a docker container with the necessary environment. 3. Run `yarn cypress:snapshot:docker "./cypress/e2e/Regression/ClientSide/Anvil/Widgets/*_spec.ts" updateSnapshots=false`. Here we can use the path to a specific file, or set `updateSnapshots=true` flag to update the screenshots. ## Automation /ok-to-test tags="@tag.Anvil" ### :mag: Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/9844579277> > Commit: 75f2659 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=9844579277&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Anvil` > Spec: > <hr>Mon, 08 Jul 2024 18:37:36 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added support for image snapshot testing with the `@types/cypress-image-snapshot` dependency. - **Refactor** - Updated test specifications by removing unnecessary `triggerInputInvalidState()` calls. - Reorganized and improved efficiency of image snapshot methods for various devices. - **Chores** - Updated `Dockerfile` to configure the Cypress environment with specific versions for dependencies. - Changed import paths in `e2e.js` for better module resolution. - **Style** - Fixed a comment typo in Cypress plugin configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Pawan Kumar <pawankumar@Pawans-MacBook-Pro-2.local> Co-authored-by: unknown <vadim@appsmith.com>
2 participants
Summary by CodeRabbit
New Features
disableScrollproperty to the table widget, allowing users to disable scrolling for tables.Bug Fixes
Tests
Style
disableScrollproperty and adjusted border styling for table rows.