NTNGL-5467 | Enable List Tile Drag and Drop#6973
Conversation
|
Thanks for the PR! 🎉 We've deployed an automatic preview for this PR - you can see your changes here:
Note The build needs to finish before your changes are deployed. |
NicholasHallman
left a comment
NicholasHallman
left a comment
There was a problem hiding this comment.
Descriptions for what some of this css is accomplishing
| } | ||
|
|
||
| :host([layout="tile"]) .d2l-list-item-drag-bottom-marker { | ||
| right: calc(-0.9rem + 3px); |
There was a problem hiding this comment.
Lots of magic numbers here but to explain, the -0.9 rem is the gap between the cards and the 3px is half of the 6px that was previously used to center the markers
There was a problem hiding this comment.
Thanks for spelling this out. I would not be adverse to putting this in a comment for "future-us". I am sure we'll be wondering how we arrived at this down the road.
| :host([layout="tile"]) .d2l-list-item-drag-drop-grid { | ||
| display: grid; | ||
| grid-template-rows: 100%; | ||
| grid-template-columns: 1rem 1fr 1fr 1rem; | ||
| } |
There was a problem hiding this comment.
This rotates the drop targets
| :host([layout="tile"]:not([is-draggable])) { | ||
| grid-template-columns: | ||
| [start outside-control-start] 0 | ||
| [outside-control-end control-start] minmax(0, min-content) | ||
| [control-end actions-start] minmax(0, auto) | ||
| [actions-end end]; |
There was a problem hiding this comment.
This varient ensures that the selection box is the left most item when the drag handle isn't present. There's a fun layout issue that prevents minmax(0, min-content) from ever equalling 0. My guess is that this is due to the content below the header spanning this column and so chrome assumes the column can't have a width of 0.
| :host([layout="tile"][draggable]) d2l-selection-input { | ||
| margin: auto; | ||
| } |
There was a problem hiding this comment.
the drag handle makes the title bar slightly taller so we need to tell the selection to center itself
There was a problem hiding this comment.
What's the height difference?
There was a problem hiding this comment.
Without the drag handles it's 39px tall, with it's 46px. So 7 pixels
There was a problem hiding this comment.
@glen-bartlett-d2l , @geurts what are your thoughts on this? I wonder if there is any way we can tighten things up so the header doesn't get thicker. Perhaps in this layout, we could render the drag handle differently so it's not quite so tall.
| this._hasListItemContent = !!this.shadowRoot.querySelector('slot:not([name])').assignedElements({ flatten: true }) | ||
| .find(elem => elem.tagName === 'D2L-LIST-ITEM-CONTENT'); | ||
| } | ||
| if (this.draggable && this.layout === 'tile' && this.tileHeader === false) { |
There was a problem hiding this comment.
Can this be simplified to if (this.draggable)?
There was a problem hiding this comment.
Yep seems like it. My thinking was that a more specific rule would keep it from running this code "by accident" but tile-head has no impact of regular list items so it looked fine in my testing with just this.draggable.
| class ListItemPlacementMarker extends LitElement { | ||
|
|
||
| static properties = { | ||
| verticle: { type: Boolean, reflect: true } |
There was a problem hiding this comment.
orientation would be another possibility, but I think vertical is fine.
Note the spelling though.
|
|
||
| .d2l-list-drag-marker-line { | ||
| height: 12px; | ||
| margin-left: -1px; |
There was a problem hiding this comment.
Don't feel like you need to change it... but we could probably update these two lines to margin-inline: -1px. I only mention it because margin-left and margin-right always raises red flags.
…paceUI/core into NTNGL-5467/tile-mouse-drag-logic
2 participants
Almost entirely css changes to re-enable drag and keyboard arrangement functionality for tiles.
The rest of the css is for properly positioning the elements and rotating the marker
Functional Tests (Demo Manual QA)
Highlighting some design decisions we made in the meeting with Glen and Tayzia