fix: Fixes editable table cell paddings for certain corner cases #3145

pan-kot · 2024-12-17T09:32:49Z

Description

The PR includes two small fixes and minor refactoring for editable table cell styles:

Improved editor vertical alignment;
Correct editable cell padding in sticky column.

Before:

Screen.Recording.2024-12-17.at.10.50.24.mov

After:

Screen.Recording.2024-12-17.at.10.49.35.mov

How has this been tested?

Manual testing on "cell-permutations" and "editable" test pages.
Screenshot tests.

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

If the code handles URLs: all URLs are validated through the checkSafeUrl function.

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.

pan-kot · 2024-12-17T09:33:51Z

src/table/body-cell/styles.scss

@@ -147,28 +148,21 @@ $cell-negative-space-vertical: 2px;
  }
  &:first-child {
    border-inline-start: $border-placeholder;
+    @include cell-padding-inline-start($cell-edge-horizontal-padding);


Moved from first-child:not(.is-visual-refresh). There is no reason to include the ":not(.is-visual-refresh)" because first column padding for VR is overridden anyways.

I think the :not() pseudo-selectors are good for clarity and for reducing overrides, and moreover these ones in particular help separate the "non-VR" styles now that VR is the default.

To me this looks like an if-else statements where if and else parts are also set apart. I think it is cleaner when some style is set as default and then another one overrides it conditionally.

I do, however, agree that VR can be the new default now. I can take a follow-up task to rework these styles so that instead of passing is-visual-refresh we will instead pass is-classic and the overrides are inverted.

pan-kot · 2024-12-17T09:34:31Z

src/table/body-cell/styles.scss

  &.is-visual-refresh:first-child {
-    &:not(.has-striped-rows) {


Removed "&:not(.has-striped-rows)" because there is an explicit override for ".has-striped-rows" down below.

pan-kot · 2024-12-17T09:35:48Z

src/table/body-cell/styles.scss

-    &.sticky-cell-pad-inline-start:not(.has-selection) {
-      @include cell-padding-inline-start($cell-horizontal-padding);
+    @include cell-padding-inline-start(awsui.$space-xxxs);
+    &.body-cell-editable:hover {


Removed the ":not(.body-cell-edit-active)" prefix because the active edit cell cannot be hovered anyways. This also fixes the bug when the wrong padding was assigned for sticky + editable + hovered cells.

codecov · 2024-12-17T09:40:18Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.40%. Comparing base (468db9e) to head (261c55f).
Report is 11 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff            @@
##             main    #3145    +/-   ##
========================================
  Coverage   96.40%   96.40%            
========================================
  Files         785      784     -1     
  Lines       22109    22131    +22     
  Branches     7586     7186   -400     
========================================
+ Hits        21314    21336    +22     
- Misses        743      788    +45     
+ Partials       52        7    -45

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

pan-kot · 2024-12-17T09:43:08Z

src/table/body-cell/styles.scss

@@ -246,9 +242,6 @@ $cell-negative-space-vertical: 2px;
      transition-duration: awsui.$motion-duration-transition-show-quick;
      transition-timing-function: awsui.$motion-easing-sticky;
    }
-    &-pad-left:not(.has-selection):not(.is-visual-refresh.body-cell:first-child.has-striped-rows) {
-      @include cell-padding-inline-start(awsui.$space-table-horizontal);


Removed this completely. The table cells should use normal paddings also when sticky.

pan-kot · 2024-12-17T09:48:01Z

src/table/body-cell/styles.scss

@@ -418,7 +411,8 @@ $cell-negative-space-vertical: 2px;
      }
      @include cell-padding-inline-start($editing-cell-padding-inline);
      @include cell-padding-inline-end($editing-cell-padding-inline);
-      @include cell-padding-block($editing-cell-padding-block);
+      @include cell-padding-block-start($editing-cell-padding-block);
+      @include cell-padding-block-end(calc($editing-cell-padding-block + 1px));


Adding a small extra space here ensures better input alignment especially in compact mode.

jperals · 2024-12-19T09:22:46Z

The inline edit field extends towards the inline-start by ~1px when hovered. This didn't happen before (see before vs after)

Untitled.mov

pan-kot · 2024-12-20T05:58:04Z

The inline edit field extends towards the inline-start by ~1px when hovered. This didn't happen before (see before vs after)

Untitled.mov

Good finding! Somehow that wasn't observed in the table itself.

I updated styles so that cell hover override is applied for multiple conditions taking the corresponding padding. That also fixed slight cell shaking in the first column in certain conditions.

Before (main):

Screen.Recording.2024-12-20.at.06.56.13.mov

After:

Screen.Recording.2024-12-20.at.06.57.16.mov

pan-kot added 2 commits December 17, 2024 09:40

fix editable cell bottom padding

8cff76e

fix editable + sticky padding

b4a3b82

pan-kot commented Dec 17, 2024

View reviewed changes

pan-kot deployed to dev-pages December 17, 2024 09:39 — with GitHub Actions View deployment

pan-kot commented Dec 17, 2024

View reviewed changes

pan-kot marked this pull request as ready for review December 17, 2024 09:52

pan-kot requested a review from a team as a code owner December 17, 2024 09:52

pan-kot requested review from jperals and removed request for a team December 17, 2024 09:52

fix first editable col shaking

261c55f

pan-kot deployed to dev-pages December 20, 2024 06:11 — with GitHub Actions View deployment

jperals approved these changes Dec 20, 2024

View reviewed changes

pan-kot added this pull request to the merge queue Dec 20, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 20, 2024

pan-kot added this pull request to the merge queue Dec 21, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 21, 2024

pan-kot added this pull request to the merge queue Dec 23, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 23, 2024

pan-kot added this pull request to the merge queue Dec 23, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 23, 2024

pan-kot added this pull request to the merge queue Dec 23, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 23, 2024

pan-kot added this pull request to the merge queue Dec 23, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 23, 2024

pan-kot added this pull request to the merge queue Dec 23, 2024

Merged via the queue into main with commit b3e4bf7 Dec 23, 2024
40 checks passed

pan-kot deleted the fix-table-cell-paddings branch December 23, 2024 12:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Fixes editable table cell paddings for certain corner cases #3145

fix: Fixes editable table cell paddings for certain corner cases #3145

pan-kot commented Dec 17, 2024 •

edited

Loading

pan-kot Dec 17, 2024

jperals Dec 18, 2024

pan-kot Dec 18, 2024

pan-kot Dec 17, 2024

pan-kot Dec 17, 2024

codecov bot commented Dec 17, 2024 •

edited

Loading

pan-kot Dec 17, 2024

pan-kot Dec 17, 2024

jperals commented Dec 19, 2024

pan-kot commented Dec 20, 2024

fix: Fixes editable table cell paddings for certain corner cases #3145

fix: Fixes editable table cell paddings for certain corner cases #3145

Conversation

pan-kot commented Dec 17, 2024 • edited Loading

Description

How has this been tested?

Correctness

Security

Testing

pan-kot Dec 17, 2024

Choose a reason for hiding this comment

jperals Dec 18, 2024

Choose a reason for hiding this comment

pan-kot Dec 18, 2024

Choose a reason for hiding this comment

pan-kot Dec 17, 2024

Choose a reason for hiding this comment

pan-kot Dec 17, 2024

Choose a reason for hiding this comment

codecov bot commented Dec 17, 2024 • edited Loading

Codecov Report

pan-kot Dec 17, 2024

Choose a reason for hiding this comment

pan-kot Dec 17, 2024

Choose a reason for hiding this comment

jperals commented Dec 19, 2024

pan-kot commented Dec 20, 2024

pan-kot commented Dec 17, 2024 •

edited

Loading

codecov bot commented Dec 17, 2024 •

edited

Loading