[looking for input] feat: List item draggable: Support interactive content in items #5742

margaree · 2025-05-23T07:51:19Z

This is to allow a user to interact with interactive elements within a list item when the only clickable functionality is that it is draggable.

I'll add comments on the general strategy. It's working well in ltr but not in rtl so looking for the following:

Is this strategy's logic sound? Is there anything that I could improve that just might happen to fix it in rtl?
If not, any suggestions?

github-actions · 2025-05-23T07:52:13Z

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-5742/

Note

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

margaree · 2025-05-23T07:51:48Z

components/list/list-item-mixin.js

+		const draggableContentAreaContet = renderDraggable ? html`
+			<div slot="content"
+				class="d2l-list-item-content"
+				id="${this._contentId}"
+				@mouseenter="${this._onMouseEnter}"
+				@mouseleave="${this._onMouseLeave}"
+				style="${styleMap({ paddingInlineStart: this._contentPaddingInlineStart })}">
+				<slot name="illustration" class="d2l-list-item-illustration">${illustration}</slot>
+				<slot>${content}</slot>
+			</div>
+		` : nothing;


This is just so I can add the styleMap to the content. Ignore for now - I'll clean this up more so that there can be only one content block.

margaree · 2025-05-23T07:55:36Z

components/list/list-item-mixin.js

@@ -773,7 +791,8 @@ export const ListItemMixin = superclass => class extends composeMixins(
 					@mouseleave="${this._onMouseLeavePrimaryAction}">
 						${primaryAction}
 				</div>` : nothing}
-				${!this._listItemInteractiveEnabled || (!primaryAction && !renderExpandableActionContent && !renderCheckboxActionContent) ? contentAreaContent : nothing}
+				${renderDraggable ? html`<div slot="content" class="d2l-draggable-content-hidden" style="${styleMap({ width: this._hiddenContentWidth })}"></div>` : nothing}


The general strategy is that this renders a content slot when draggable (which wouldn't have been rendered here) in order to be able to properly have the grid size all the other grid columns.

What happens is:

after initial render, this hidden content gets its width set to that of the actual visible content

~~this triggers the grid to update sizes and we get the offsetLeft of the hidden content~~

we set the padding-inline-start of the visible content to ~~the value of the offsetLeft of the hidden content (since it's in the outside-content-action slot which is full list item width)~~ a value dependant on if expandable and/or color slots are present.

This does generally trigger another update of the hidden content width because the width of the content gets updated.

Without this hidden div:

Yeah, the extra rendering is the opposite direction I was hoping list-items to go. 😢

margaree · 2025-05-23T07:56:18Z

components/list/list-item-mixin.js

@@ -744,7 +762,7 @@ export const ListItemMixin = superclass => class extends composeMixins(
 				<div slot="before-content"></div>
 				${this._renderDropTarget()}
 				${this._renderDragHandle(this._renderOutsideControl)}
-				${this._renderDragTarget(this.dragTargetHandleOnly ? this._renderOutsideControlHandleOnly : this._renderOutsideControlAction)}
+				${this._renderDragTarget(this.dragTargetHandleOnly ? this._renderOutsideControlHandleOnly : this._renderOutsideControlAction, renderDraggable ? draggableContentAreaContet : null)}


This otherwise follows the same strategy as used in the other clickable places - the content is rendered within the clickable draggable area in order to be able to click the interactive elements.

margaree · 2025-05-23T07:57:17Z

components/list/list-item-drag-drop-mixin.js

+			if (!hiddenItem) return;
+
+			const offsetLeft = hiddenItem.offsetLeft;
+			this._contentPaddingInlineStart = this.#isRTL() ? `${-1 * offsetLeft}px` : `${offsetLeft}px`;


rtl on the demo page is entering an infinite loop which sets the padding to 0 and back to the expected value.

margaree · 2025-05-23T07:58:15Z

components/list/list-item-drag-drop-mixin.js

+	}
+
+	#updateContentPadding() {
+		setTimeout(() => {


using setTimeout rather than requestAnimationFrame seemed to allow enough time for things to be sized such that we can avoid an extra render.

margaree · 2025-05-23T07:59:51Z

components/list/list-item-drag-drop-mixin.js

+			this._hiddenContentWidth = `${width}px`;
+		});
+
+		this._contentResizeObserver.observe(content);


There is some tweaking that can be done such that after firstUpdated we can get the padding and width and avoid the resize observer being necessary initially.

However, in practice I found that there were a lot of cases where things sized too late for that and we still needed the resized observer to get the spacing all correct.

margaree · 2025-05-23T08:00:28Z

components/list/list-item-mixin.js

+			_hiddenContentWidth: { state: true },
+			_contentPaddingInlineStart: { state: true },


These could get moved to drag-drop-mixin.

margaree · 2025-06-13T10:18:10Z

components/list/list-item-drag-drop-mixin.js

+			[slot="outside-control-action"] [slot="content"] {
+				padding-inline-start: 1.5rem;  /* left and right margins of 0.3rem + drag handle width of 0.9rem */
+			}
+			:host([_render-expand-collapse-slot]) [slot="outside-control-action"] [slot="content"] {
+				padding-inline-start: 3rem; /* draggable padding + 1.2rem wide + 0.3rem padding */
+			}
+			:host([_has-color-slot]) [slot="outside-control-action"] [slot="content"] {
+				padding-inline-start: calc(1.5rem + 12px + var(--d2l-list-item-color-width, 6px));  /* draggable padding + 12px color padding + color width  */
+			}
+			:host([_render-expand-collapse-slot][_has-color-slot]) [slot="outside-control-action"] [slot="content"] {
+				padding-inline-start: calc(3rem + 12px + var(--d2l-list-item-color-width, 6px) - 6px); /* all of the above - 6px when color + expand */
+			}


This hard codes the inline-start spacing the content would need to line up with its siblings/be properly placed under a parent.

The previous strategy was to calculate this based on the hidden content item. However, that could cause infinite loops.

This method is fragile in the event those other paddings change, but I believe that would get caught by vdiffs.

margaree · 2025-06-13T10:19:26Z

components/list/list-item-mixin.js

+				padding-block: 0.55rem;
+				padding-inline: 0 0.55rem;


Updated this in order to be able to more easily override it in draggable mixin.

margaree · 2025-06-13T10:20:04Z

components/list/list-item-mixin.js

@@ -773,7 +791,8 @@ export const ListItemMixin = superclass => class extends composeMixins(
 					@mouseleave="${this._onMouseLeavePrimaryAction}">
 						${primaryAction}
 				</div>` : nothing}
-				${!this._listItemInteractiveEnabled || (!primaryAction && !renderExpandableActionContent && !renderCheckboxActionContent) ? contentAreaContent : nothing}
+				${renderDraggable ? html`<div slot="content" class="d2l-draggable-content-hidden" style="${styleMap({ width: this._hiddenContentWidth })}"></div>` : nothing}


Without this hidden div:

margaree · 2025-06-13T10:21:39Z

components/list/list-item-drag-drop-mixin.js

@@ -883,7 +928,7 @@ export const ListItemDragDropMixin = superclass => class extends superclass {
 		return this._dragMultiple && this.draggable && (this.selectable || this.expandable) ? html`<d2l-list-item-drag-image></d2l-list-item-drag-image>` : nothing;
 	}

-	_renderDragTarget(templateMethod) {
+	_renderDragTarget(templateMethod, content) {


This piece is the same as the other cases, where content goes in the clickable area in order to be able to interact with it.

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

dbatiste · 2025-06-16T14:06:46Z

Just a brief update on this: Margaree and I spoke about this issue and the changes. Moving the content slot such that the grid no longer works as we'd expect isn't super desirable, nor is adding the extra observer to each draggable item. I proposed the idea of not supporting "drag anywhere" capability when the items contain interactive content with the hope that might avoid the need for this. Margaree to bounce the idea off Jeff.

margaree · 2025-06-17T06:51:29Z

At this point we will just be recommending that for cases where there is interactive content within a list item and where the list item is only draggable then the consumer should either use the drag-target-handle-only attribute on the list item or put the interactive content in the actions slot.

I will be creating a separate PR with this documentation.

margaree added 2 commits May 21, 2025 14:53

initial work on list item with draggable and interactive content

6298166

simplify resizeobserver strategy

ee6df99

github-actions bot mentioned this pull request May 23, 2025

Updating vdiff goldens for PR 5742 #5744

Merged

margaree commented May 23, 2025

View reviewed changes

margaree marked this pull request as ready for review May 23, 2025 08:26

margaree requested a review from a team as a code owner May 23, 2025 08:26

margaree added 5 commits June 13, 2025 08:16

Merge branch 'main' into list-item-drag-drop-interactive-content

05f5712

add demo

6c3ef8f

use hard coded padding values instead

89c650a

add unit tests

3925e1a

add vdiff

510a3ed

margaree commented Jun 13, 2025

View reviewed changes

Updating vdiff goldens (#5744)

ed060a4

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

github-actions bot mentioned this pull request Jun 13, 2025

Updating vdiff goldens for PR 5742 #5867

Closed

margaree closed this Jun 17, 2025

		_hiddenContentWidth: { state: true },
		_contentPaddingInlineStart: { state: true },

[looking for input] feat: List item draggable: Support interactive content in items #5742

[looking for input] feat: List item draggable: Support interactive content in items #5742

Uh oh!

Conversation

margaree commented May 23, 2025

Uh oh!

github-actions bot commented May 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

margaree May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dbatiste commented Jun 16, 2025

Uh oh!

margaree commented Jun 17, 2025

Uh oh!

Uh oh!

margaree May 23, 2025 •

edited

Loading