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.
…paceUI/core into NTNGL-5467/tile-mouse-drag-logic
| </template> | ||
| </d2l-demo-snippet> | ||
|
|
||
| <h1>Tile Drag and Drop</h1> |
There was a problem hiding this comment.
It's just a demo page, but the demo page's h1 is rendered further above. I'd either either just render all h2s (updating the heading text to be tile specific), or I would restructure the headings for all the demos on this page to use h2s and h3s.
|
Now with side to side move buttons! LTR RTL |
* Updating translations - Sync * Updating translations - Sort --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…paceUI/core into NTNGL-5467/tile-mouse-drag-logic
Yeah I agree it is a bit much. One alternative would be to make the buttons 24x24 pixels so it's less awkward? |
@glen-bartlett-d2l what do you think we should do here? Happy to hook up to discuss options. |
@dbatiste @NicholasHallman Yeah I think increasing the button size is the way to go. Here’s what it should look like, including the new measurements. I think we can also tighten up the space between the drag button / move button and the checkbox, we're been using 6px space for button spacing in the top bar 
|
@NicholasHallman Not sure if this is just artifacts from the capture, but is there a small gap on the top and the right of the card between the background and the border? 
|
|
@glen-bartlett-d2l That looks great I'll get right on it. Also about the spacing, I noticed that too a bit ago. I believe that was happening before my changes but I'll take a closer look just to be sure. If it's unrelated I can take a look at fixing it as part of a seperate pr but otherwise I'll wrangle it here :) |
|
I think that's a separate thing. Not sure if it's just an artifact of the zoom and how the grid items are laid out. Would love a fix if you can solve it easily. Otherwise I will look into it / or create a ticket for us. |
New bigger buttons! Also tightened up the margins so the spacing between the selection box and the drag handle is the same as the drag handle to the edge of the card.
Took a peek, it's a pesky issue. The slotted content seems to overlay with the borders so some css is shrinking everything by 1px. When we have off layouts with subpixels (like the 800.4 px wide card above) you can see some white spacing. |
I was afraid that might be the case. 😢 |
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
3 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