Fix: Gradient overlay on TableCard does not disappear on resize #60208
Conversation
…es-not-disappear-on-resize
Testing GuidelinesHi @woocommerce/gamma, Apart from reviewing the code changes, please make sure to review the testing instructions (Guide) and verify that relevant tests (E2E, Unit, Integration, etc.) have been added or updated as needed. Reminder: PR reviewers are required to document testing performed. This includes:
|
📝 WalkthroughWalkthroughA patch was introduced to fix a bug where a linear gradient overlay did not disappear on table resize. The Table component's scroll shadow logic and resize handling were refactored for clarity and correctness. A new wide-table story was added for horizontal scrolling, and a test was introduced to verify scrollable class removal. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Window
participant TableComponent
User->>Window: Resize event
Window->>TableComponent: handleResize()
TableComponent->>TableComponent: requestAnimationFrame(updateTableShadow)
TableComponent->>TableComponent: Check scrollWidth vs offsetWidth
alt Not scrollable
TableComponent->>TableComponent: Remove scroll shadows, reset scrollLeft
else Scrollable
TableComponent->>TableComponent: Set scroll shadows based on position
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/js/components/src/table/test/index.js (1)
277-277: Remove unnecessary empty object destructuring.The empty object pattern is not needed here since no values are being extracted from the render result.
Apply this diff to fix the issue:
- const {} = render( + render(
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/js/components/changelog/fix-woopmnt-5217-linear-gradient-overlay-does-not-disappear-on-resize(1 hunks)packages/js/components/src/table/stories/table-card.story.tsx(2 hunks)packages/js/components/src/table/table.tsx(2 hunks)packages/js/components/src/table/test/index.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{php,js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/code-quality.mdc)
**/*.{php,js,jsx,ts,tsx}: Guard against unexpected inputs
Sanitize and validate any potentially dangerous inputs
Ensure code is backwards compatible
Write code that is readable and intuitive
Ensure code has unit or E2E tests where applicable
Files:
packages/js/components/src/table/test/index.jspackages/js/components/src/table/table.tsxpackages/js/components/src/table/stories/table-card.story.tsx
**/*.{php,js,ts,jsx,tsx}
⚙️ CodeRabbit Configuration File
**/*.{php,js,ts,jsx,tsx}: Don't trust that extension developers will follow the best practices, make sure the code:
- Guards against unexpected inputs.
- Sanitizes and validates any potentially dangerous inputs.
- Is backwards compatible.
- Is readable and intuitive.
- Has unit or E2E tests where applicable.
Files:
packages/js/components/src/table/test/index.jspackages/js/components/src/table/table.tsxpackages/js/components/src/table/stories/table-card.story.tsx
🧠 Learnings (1)
📓 Common learnings
Learnt from: nerrad
PR: woocommerce/woocommerce#60176
File: plugins/woocommerce/client/legacy/css/admin.scss:9088-9105
Timestamp: 2025-08-04T00:21:51.440Z
Learning: WooCommerce is currently in the midst of an admin redesign project, so temporary/short-term CSS solutions for admin pages are preferred over long-term refactoring when addressing immediate needs.
Learnt from: Aljullu
PR: woocommerce/woocommerce#59370
File: plugins/woocommerce/client/blocks/assets/js/blocks/product-gallery/style.scss:236-243
Timestamp: 2025-07-03T10:09:20.635Z
Learning: In WooCommerce blocks CSS, prefer using standard CSS properties like `scrollbar-width` over vendor-specific prefixes when the standard property has good browser support. As of late 2024, `scrollbar-width` is widely supported across all major browsers and doesn't require WebKit-specific workarounds.
Learnt from: samueljseay
PR: woocommerce/woocommerce#58716
File: plugins/woocommerce/client/blocks/assets/js/blocks/mini-cart/iapi-frontend.ts:83-101
Timestamp: 2025-06-17T07:07:53.443Z
Learning: In WooCommerce blocks, when porting existing code patterns that have known issues (like parseInt truncating decimal money values), maintain consistency with existing implementation rather than making isolated fixes. The preference is for systematic refactoring approaches (like broader Dinero adoption) over piecemeal changes.
Learnt from: NeosinneR
PR: woocommerce/woocommerce#0
File: :0-0
Timestamp: 2025-06-26T14:25:08.421Z
Learning: In WooCommerce transactional emails, product images have historically had display issues due to lazy loading attributes being applied, which email clients cannot process since they don't execute JavaScript. This issue existed in both old and new email templates, but became more visible with the new email template system. The fix involves preventing lazy loading on attachment images specifically during email generation by adding skip classes and data attributes.
Learnt from: ralucaStan
PR: woocommerce/woocommerce#58782
File: plugins/woocommerce/client/blocks/assets/js/base/utils/render-frontend.tsx:0-0
Timestamp: 2025-06-16T16:12:12.148Z
Learning: For WooCommerce checkout blocks, lazy loading was removed in favor of direct imports to prevent sequential "popping" effects during component loading. This approach prioritizes user experience over code splitting, with minimal bundle size impact and improved performance (1.7s to 1.1s speed score improvement). The checkout flow benefits from having all components load together rather than incrementally.
Learnt from: CR
PR: woocommerce/woocommerce#0
File: .cursor/rules/generate-pr-description.mdc:0-0
Timestamp: 2025-07-21T05:22:46.426Z
Learning: Provide clear, step-by-step instructions for how to test the changes in the PR description.
Learnt from: lysyjan
PR: woocommerce/woocommerce#59632
File: packages/js/email-editor/src/layouts/flex-email.tsx:116-122
Timestamp: 2025-07-14T10:41:46.200Z
Learning: In WooCommerce projects, formatting suggestions should respect the project's Prettier configuration and linting rules. Changes that would break the lint job should be avoided, even if they appear to improve readability.
Learnt from: dinhtungdu
PR: woocommerce/woocommerce#59900
File: plugins/woocommerce/client/blocks/assets/js/blocks/product-filters/inner-blocks/attribute-filter/inspector.tsx:0-0
Timestamp: 2025-07-24T05:37:00.907Z
Learning: The DisplayStyleSwitcher component in plugins/woocommerce/client/blocks/assets/js/blocks/product-filters/components/display-style-switcher/index.tsx has been updated so that its onChange prop accepts only a string type (not string | number | undefined), eliminating the need for type assertions when using this component.
🪛 Biome (2.1.2)
packages/js/components/src/table/test/index.js
[error] 277-277: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
⏰ 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). (9)
- GitHub Check: build
- GitHub Check: Check Asset Sizes
- GitHub Check: label_project
- GitHub Check: Core e2e tests 1/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Metrics - @woocommerce/plugin-woocommerce [performance]
- GitHub Check: Core e2e tests 2/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Core e2e tests 5/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Core e2e tests 3/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Core e2e tests 4/6 - @woocommerce/plugin-woocommerce [e2e]
🔇 Additional comments (8)
packages/js/components/changelog/fix-woopmnt-5217-linear-gradient-overlay-does-not-disappear-on-resize (1)
1-4: LGTM! Well-formatted changelog entry.The changelog entry correctly follows the standard format with appropriate significance level (patch) and type (fix), with a clear description of the issue being resolved.
packages/js/components/src/table/test/index.js (1)
262-320: Excellent test coverage for the resize scenario.This test effectively validates the core fix by:
- Mocking scrollable table dimensions
- Simulating window resize events
- Verifying that scrollable CSS classes are properly removed when table becomes non-scrollable
- Testing the exact scenario described in the PR objectives
The test approach of mocking DOM properties and dispatching events is appropriate for this use case.
packages/js/components/src/table/stories/table-card.story.tsx (3)
13-65: Well-structured test data for horizontal scrolling.The wide table headers and rows provide comprehensive test data with 10 columns covering various financial metrics. This will effectively demonstrate the horizontal scrolling behavior and gradient overlay functionality.
181-205: Excellent addition for testing the fix.The
TableCardWideExamplestory provides a practical way to test the horizontal scrolling scenario described in the PR objectives. The implementation follows the established pattern with proper state management.
210-210: Good export naming for the wide table story.The
WideTableexport name is clear and will be easily identifiable in Storybook for testing the fix.packages/js/components/src/table/table.tsx (3)
132-153: Excellent refactor with clear early exit logic.The new logic significantly improves clarity by:
- Adding explicit null check and early return
- Clearly identifying when table is not scrollable
- Resetting both scroll indicators and position when table becomes non-scrollable
- Eliminating the complex nested conditional logic from the previous implementation
This directly addresses the core issue where scroll indicators weren't being cleared on resize.
155-162: Clean and explicit scroll state management.The scroll state calculation is now much more readable with clear variable names and straightforward boolean logic. This makes the code easier to maintain and debug.
184-195: Smart use of requestAnimationFrame for resize handling.Using
requestAnimationFrameensures that the shadow update happens after DOM layout changes are complete, which is crucial for accurate dimension calculations during resize events. This timing improvement likely resolves edge cases where the gradient overlay wouldn't update correctly.
|
Size Change: +200 B (0%) Total Size: 5.92 MB
|
Test using WordPress PlaygroundThe changes in this pull request can be previewed and tested using a WordPress Playground instance. Test this pull request with WordPress Playground. Note that this URL is valid for 30 days from when this comment was last updated. You can update it by closing/reopening the PR or pushing a new commit. |
…es-not-disappear-on-resize
…es-not-disappear-on-resize
…es-not-disappear-on-resize
…es-not-disappear-on-resize
Submission Review Guidelines:
Changes proposed in this Pull Request:
This PR fixes a bug where the linear gradient overlay on tables would not disappear when resizing the viewport back to a larger size. The issue was affecting tables in the WooPayments plugin.
The
updateTableShadow()function in the table component was not properly detecting when a table became non-scrollable after resize, causing the scroll indicators (is-scrollable-rightandis-scrollable-leftclasses) to remain active, which kept the linear gradient visible.Closes #WOOPMNT-5217.
Screenshots or screen recordings:
How to test the changes in this Pull Request:
Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:
pnpm --filter @woocommerce/storybook run watch:build:storybookTesting that has already taken place:
Please check the section above
Changelog entry
Changelog Entry Details
Significance
Type
Message
Changelog Entry Comment
Comment