Table Updates: Styles #4671

margaree · 2024-05-10T12:57:24Z

Due to the potential downstream impact of table style changes, I wanted to get hopefully all of them in one PR, so it is a bit large.

This PR covers the following stories:

In summary, we want to do the following:

Update the look and feel of the d2l-table-col-sort-button to take up the whole cell if it can (nothing else in the cell) and have fallback styles if not (10px padding on each side in the shared cell case)
All cells should have 0.7rem of padding

github-actions · 2024-05-10T12:58:58Z

Thanks for the PR! 🎉

We've deployed an automatic preview for this PR - you can see your changes here:

URL	https://live.d2l.dev/prs/BrightspaceUI/core/pr-4671/

Note

The build needs to finish before your changes are deployed.
Changes to the PR will automatically update the instance.

components/table/table-col-sort-button.js

…event movement when button goes from nosort to sort

… into table-updates-styles4

margaree

Nicole is interested in having it so that if there are multiple things in the cell (e.g., multiple table-col-sort-button, table-col-sort-button + button-icon) that when they wrap, if one is focused and the other is hovered then those two styles just touch in order to reduce height. I have found trying to do this to be difficult (perhaps I'm missing something totally obvious) so let me know if you have suggestions on that. Figured this out

margaree · 2024-05-13T15:33:55Z

components/table/table-wrapper.js

+	d2l-button-icon {
+		--d2l-button-icon-min-height: calc(var(--d2l-table-cell-overall-height) - 2 * var(--d2l-table-cell-col-sort-button-size-offset));
+		--d2l-button-icon-min-width: calc(var(--d2l-table-cell-overall-height) - 2 * var(--d2l-table-cell-col-sort-button-size-offset));
+	}


We want button-icons to be the same size as the d2l-table-col-sort-button.
Considerations:

d2l-dropdown-context-menu would not be targeted by this

This does go against letting consumers control their own styles for the content of the table cells

margaree · 2024-05-13T18:07:01Z

components/table/table-wrapper.js

+	.d2l-checkbox,
+	d2l-input-checkbox,
+	d2l-selection-select-all,
+	d2l-selection-input {
+		margin-block: calc(0.5 * (var(--d2l-table-cell-height) - ${cssSizes.inputBoxSize}rem));
+	}
+	.d2l-table > * > tr.d2l-table-selected-first d2l-input-checkbox,
+	.d2l-table > * > tr.d2l-table-selected-first d2l-selection-input,
+	.d2l-table > * > tr.d2l-table-selected-first .d2l-checkbox {
+		margin-bottom: calc(0.5 * (var(--d2l-table-cell-height) - ${cssSizes.inputBoxSize}rem));
+		margin-top: calc(0.5 * (var(--d2l-table-cell-height) - ${cssSizes.inputBoxSize}rem) - 1px);
+	}
+	@supports selector(:has(a, b)) {
+		.d2l-table > * > tr > :has(.d2l-checkbox),
+		.d2l-table > * > tr > :has(d2l-selection-select-all),
+		.d2l-table > * > tr > :has(d2l-input-checkbox),
+		.d2l-table > * > tr > :has(d2l-selection-input) {
+			padding-block: calc(0.5 * (var(--d2l-table-cell-overall-height) - ${cssSizes.inputBoxSize}rem));
+		}
+		.d2l-table > * > tr.d2l-table-selected-first > :has(.d2l-checkbox),
+		.d2l-table > * > tr.d2l-table-selected-first > :has(d2l-input-checkbox),
+		.d2l-table > * > tr.d2l-table-selected-first > :has(d2l-selection-input) {
+			padding-bottom: calc(0.5 * (var(--d2l-table-cell-overall-height) - ${cssSizes.inputBoxSize}rem));
+			padding-top: calc(0.5 * (var(--d2l-table-cell-overall-height) - ${cssSizes.inputBoxSize}rem) - 1px);
+		}
+	}


The checkbox inputs add extra height that we don't want so we shrink the padding accordingly.

margaree · 2024-05-13T18:12:27Z

components/table/table-wrapper.js

+		--d2l-table-col-sort-button-additional-padding-inline-end: 0px; /* stylelint-disable-line length-zero-no-unit */
+	}
+	d2l-table-col-sort-button:not(:only-child):first-child {
+		--d2l-table-col-sort-button-margin-inline-end: 1px;


Minimize space between d2l-table-col-sort-buttons on siblings (default is 4px)

margaree · 2024-05-13T18:16:08Z

components/table/table-wrapper.js

-				--d2l-table-cell-height: 1.15rem; /* min-height to be 48px including border */
-				--d2l-table-cell-padding: 0.6rem;
-				--d2l-table-cell-padding-alt: calc(0.6rem - 1px) 0.6rem 0.6rem 0.6rem;


Light type padding and height is now to be consistent with default

margaree · 2024-05-13T18:29:28Z

components/table/table-col-sort-button.js

+				--d2l-table-col-sort-button-width: calc(100% - 2 * var(--d2l-table-cell-col-sort-button-size-offset));
+			}
+			:host([nosort]) {
+				--d2l-table-col-sort-button-additional-padding-inline-end: calc(0.6rem + 18px);


This is to account for extra space for the icon + icon's margin when the button is in nosort state. It's a cssvar in order to be able to set it for different cases when there are siblings.

margaree · 2024-05-13T18:32:31Z

components/table/table-wrapper.js

+		padding-inline-end: calc(0.6rem + 18px);
+	}
+	.d2l-table-header-col-sortable :not(d2l-table-col-sort-button) {
+		margin: var(--d2l-table-cell-col-sort-button-size-offset);


This adds a margin to non-table-col-sort-buttons that are in a cell with one. In our demos it looks weird without this, however consumers can be setting their own spacing on their cell contents so I don't know that it's right for us to be dictating that. Let me know any thoughts. I'm open to removing it from here and moving it elsewhere for the demo.

Yeah this seems a bit invasive to me, and likely best handled by consumer code wherever it comes up.

margaree · 2024-05-13T18:33:26Z

components/table/table-wrapper.js

@@ -368,6 +437,7 @@ export class TableWrapper extends RtlMixin(PageableMixin(SelectionMixin(LitEleme
 			r.classList.toggle('d2l-table-selected-first', firstNonHeaderRow && isSelected);

 			Array.from(r.cells).forEach((c, index) => {
+				if (isHeader && !CSS.supports('selector(:has(a, b))')) this._checkSiblingSortableCells(c);


With :has we are able to do everything without js, so that's why there is a separate :has section in the css. This can be removed once we only support browsers with :has

margaree · 2024-05-13T18:48:04Z

components/table/table-wrapper.js

+				--d2l-table-cell-height: calc(var(--d2l-table-cell-overall-height) - 2 * var(--d2l-table-cell-padding));
+				--d2l-table-cell-padding: 0.7rem;
+				--d2l-table-cell-padding-alt: calc(0.7rem - 1px) 0.7rem 0.7rem 0.7rem;
+				--d2l-table-cell-col-sort-button-size-offset: 4px;


This corresponds to the 4px of buffer we want on all sides of the table-col-sort-button

dlockhart

Crazy how complex all the positioning ended up being, nice job!

dlockhart · 2024-05-14T18:55:36Z

components/table/table-wrapper.js

+		padding-inline-end: calc(0.6rem + 18px);
+	}
+	.d2l-table-header-col-sortable :not(d2l-table-col-sort-button) {
+		margin: var(--d2l-table-cell-col-sort-button-size-offset);


Yeah this seems a bit invasive to me, and likely best handled by consumer code wherever it comes up.

… one and move into demo

…ther items, tweak demo table

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

…ck for --d2l-table-cell-col-sort-button-size-offset css var

d2l-github-release-tokens · 2024-05-27T18:25:11Z

🎉 This PR is included in version 3.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Table Updates: Styles

5c20e89

github-actions bot mentioned this pull request May 10, 2024

Updating vdiff goldens for PR 4671 #4672

Merged

dlockhart reviewed May 10, 2024

View reviewed changes

components/table/table-col-sort-button.js Show resolved Hide resolved

margaree added 7 commits May 10, 2024 14:36

design feedback: when multiple items in cell, put closer together, pr…

35a206e

…event movement when button goes from nosort to sort

design feedback: try out reducing button padding

c5edb68

general clean up, add margin to siblings of d2l-table-col-sort-button

59b34d3

Merge branch 'main' into table-updates-styles4

52e5c5a

pre-PR cleanup

f8bb7ea

Merge branch 'table-updates-styles4' of github.com:BrightspaceUI/core…

ca520a5

… into table-updates-styles4

more pre-PR cleanup

2de3f86

margaree commented May 13, 2024

View reviewed changes

margaree marked this pull request as ready for review May 13, 2024 18:54

margaree requested a review from a team as a code owner May 13, 2024 18:54

Make the spacing smaller between wrapped items

45ae8af

dlockhart approved these changes May 14, 2024

View reviewed changes

margaree and others added 4 commits May 14, 2024 15:47

remove styles targeting non-d2l-table-col-sort-button in same cell as…

02397f8

… one and move into demo

design feedback: remove space between d2l-table-col-sort-button and o…

618d720

…ther items, tweak demo table

Merge branch 'main' into table-updates-styles4

94dcc5b

Updating vdiff goldens (#4672)

ea5684e

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

github-actions bot mentioned this pull request May 27, 2024

Updating vdiff goldens for PR 4671 #4698

Closed

be more specific about d2l-button-icon and checkbox css, add a fallba…

3d41e84

…ck for --d2l-table-cell-col-sort-button-size-offset css var

margaree merged commit 7137456 into main May 27, 2024
6 checks passed

margaree deleted the table-updates-styles4 branch May 27, 2024 18:23

d2l-github-release-tokens bot added the released label May 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Table Updates: Styles #4671

Table Updates: Styles #4671

margaree commented May 10, 2024 •

edited

github-actions bot commented May 10, 2024

margaree left a comment •

edited

margaree May 13, 2024

margaree May 13, 2024

margaree May 13, 2024

margaree May 13, 2024

margaree May 13, 2024

margaree May 13, 2024

dlockhart May 14, 2024

margaree May 13, 2024

margaree May 13, 2024

dlockhart left a comment

dlockhart May 14, 2024

d2l-github-release-tokens bot commented May 27, 2024

Table Updates: Styles #4671

Table Updates: Styles #4671

Conversation

margaree commented May 10, 2024 • edited

github-actions bot commented May 10, 2024

margaree left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dlockhart left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

d2l-github-release-tokens bot commented May 27, 2024

margaree commented May 10, 2024 •

edited

margaree left a comment •

edited