Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Allows table cells be expandable and editable at the same time #3205

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pan-kot
Copy link
Member

@pan-kot pan-kot commented Jan 17, 2025

Description

The PR makes it possible to have cell editing in the first column of tables with expandable rows.

Rel: [tNAkA40KWyvj], AWSUI-43194.

How has this been tested?

  • New unit and integration test
  • Screenshot tests
  • Dry-run to live
Review checklist

The following items are to be evaluated by the author(s) and the reviewer(s).

Correctness

  • Changes include appropriate documentation updates.
  • Changes are backward-compatible if not indicated, see CONTRIBUTING.md.
  • Changes do not include unsupported browser features, see CONTRIBUTING.md.
  • Changes were manually tested for accessibility, see accessibility guidelines.

Security

Testing

  • Changes are covered with new/existing unit tests?
  • Changes are covered with new/existing integration tests?

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Copy link

codecov bot commented Jan 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.45%. Comparing base (89ffd99) to head (b41676b).
Report is 25 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3205      +/-   ##
==========================================
+ Coverage   96.42%   96.45%   +0.03%     
==========================================
  Files         787      791       +4     
  Lines       22190    22310     +120     
  Branches     7206     7255      +49     
==========================================
+ Hits        21396    21520     +124     
+ Misses        787      783       -4     
  Partials        7        7              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pan-kot pan-kot force-pushed the fix-table-expandable-editable-cells branch from 0595994 to a93a8b2 Compare January 17, 2025 15:20
@pan-kot pan-kot force-pushed the fix-table-expandable-editable-cells branch from a93a8b2 to 985037b Compare January 17, 2025 15:43
@pan-kot pan-kot force-pushed the fix-table-expandable-editable-cells branch from 985037b to d7ea97d Compare January 17, 2025 19:16
@pan-kot pan-kot force-pushed the fix-table-expandable-editable-cells branch from b58991d to 408ab12 Compare January 26, 2025 08:07
@pan-kot pan-kot force-pushed the fix-table-expandable-editable-cells branch from 408ab12 to 57c7b25 Compare January 26, 2025 08:51
@pan-kot pan-kot marked this pull request as ready for review January 27, 2025 14:02
@pan-kot pan-kot requested a review from a team as a code owner January 27, 2025 14:02
@pan-kot pan-kot requested review from at-susie, a team and michaeldowseza and removed request for a team and at-susie January 27, 2025 14:02
@pan-kot pan-kot marked this pull request as draft January 28, 2025 10:39
@pan-kot pan-kot force-pushed the fix-table-expandable-editable-cells branch 2 times, most recently from 219b1ce to 88a8e82 Compare January 28, 2025 13:21
@pan-kot pan-kot requested a review from gethinwebster January 28, 2025 19:01
@pan-kot pan-kot force-pushed the fix-table-expandable-editable-cells branch from 88a8e82 to 270bd64 Compare January 29, 2025 08:35
gethinwebster
gethinwebster previously approved these changes Jan 29, 2025
@pan-kot pan-kot marked this pull request as ready for review January 29, 2025 09:24
@michaeldowseza michaeldowseza removed their request for review January 29, 2025 15:25
@pan-kot pan-kot force-pushed the fix-table-expandable-editable-cells branch from 270bd64 to b41676b Compare January 29, 2025 15:29
@pan-kot
Copy link
Member Author

pan-kot commented Jan 29, 2025

@gethinwebster, I added a small refactoring on top, removing the onMouseEnter/onMouseExit handlers from the cell. Apologies for not creating a separate commit for that!

The reason the handlers are removed is the following:

  1. After the original change, the handlers were used to set the hover state used as a condition to render the success icon;
  2. The success icon renders if either hover or focus is present;
  3. The focus is always present after the field is submitted, so the hover dependency is not actually needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants