Skip to content

Added Drag & Drop Reordering to Shopping List Card. #7296

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bramkragten merged 11 commits into from
Jan 5, 2021
Merged

Added Drag & Drop Reordering to Shopping List Card. #7296

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
f795a89
Added drag & drop reordering to shopping list card.
ShaneQi Oct 10, 2020
7a4c322
Migrated shopping list card icons from <ha-icon> to <ha-svg-icon>.
ShaneQi Oct 10, 2020
212bd40
Removed an unnecessary `Promise.all`.
ShaneQi Oct 10, 2020
fc00eda
Used @query to cache sortable element in shopping list card.
ShaneQi Oct 11, 2020
3aa34f2
Cleaned up syntax.
ShaneQi Oct 11, 2020
82e40e7
Merge branch 'dev' into feature/drag-and-drop-in-shopping-list
ShaneQi Oct 23, 2020
b3af653
Fixed duplicated imports.
ShaneQi Oct 23, 2020
20d083d
Moved up absolute imports above relative imports in shopping list card.
ShaneQi Oct 28, 2020
b91412f
Removed sortableEl cache in shopping list card.
ShaneQi Nov 17, 2020
99caed8
Fixed a edge case by observing sortable onEnd event instead of onSort.
ShaneQi Nov 27, 2020
510c61f
Fixed a lint issue.
ShaneQi Nov 27, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/data/shopping-list.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,12 @@ export const addItem = (
type: "shopping_list/items/add",
name,
});

export const reorderItems = (
hass: HomeAssistant,
itemIds: [string]
): Promise<ShoppingListItem> =>
hass.callWS({
type: "shopping_list/items/reorder",
item_ids: itemIds,
});
149 changes: 124 additions & 25 deletions src/panels/lovelace/cards/hui-shopping-list-card.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,12 @@ import {
property,
PropertyValues,
TemplateResult,
query,
} from "lit-element";
import { classMap } from "lit-html/directives/class-map";
import { repeat } from "lit-html/directives/repeat";
import { guard } from "lit-html/directives/guard";
import { mdiDrag, mdiSort, mdiPlus, mdiNotificationClearAll } from "@mdi/js";
import { applyThemesOnElement } from "../../../common/dom/apply_themes_on_element";
import "../../../components/ha-card";
import "../../../components/ha-icon";
Expand All @@ -23,12 +26,15 @@ import {
fetchItems,
ShoppingListItem,
updateItem,
reorderItems,
} from "../../../data/shopping-list";
import { SubscribeMixin } from "../../../mixins/subscribe-mixin";
import { HomeAssistant } from "../../../types";
import { LovelaceCard, LovelaceCardEditor } from "../types";
import { SensorCardConfig, ShoppingListCardConfig } from "./types";

let Sortable;

@customElement("hui-shopping-list-card")
class HuiShoppingListCard extends SubscribeMixin(LitElement)
implements LovelaceCard {
Expand All @@ -51,6 +57,14 @@ class HuiShoppingListCard extends SubscribeMixin(LitElement)

@internalProperty() private _checkedItems?: ShoppingListItem[];

@internalProperty() private _reordering = false;

@internalProperty() private _renderEmptySortable = false;

private _sortable?;

@query("#sortable", true) private _sortableEl?: HTMLElement;
Comment thread
ShaneQi marked this conversation as resolved.
Outdated

public getCardSize(): number {
return (this._config ? (this._config.title ? 2 : 0) : 0) + 3;
}
Expand Down Expand Up @@ -103,15 +117,15 @@ class HuiShoppingListCard extends SubscribeMixin(LitElement)
})}
>
<div class="addRow">
<ha-icon
<ha-svg-icon
class="addButton"
icon="hass:plus"
.path=${mdiPlus}
.title=${this.hass!.localize(
"ui.panel.lovelace.cards.shopping-list.add_item"
)}
@click=${this._addItem}
>
</ha-icon>
</ha-svg-icon>
<paper-input
no-label-float
class="addBox"
Expand All @@ -120,28 +134,27 @@ class HuiShoppingListCard extends SubscribeMixin(LitElement)
)}
@keydown=${this._addKeyPress}
></paper-input>
<ha-svg-icon
class="reorderButton"
.path=${mdiSort}
.title=${this.hass!.localize(
"ui.panel.lovelace.cards.shopping-list.reorder_items"
)}
@click=${this._toggleReorder}
>
</ha-svg-icon>
</div>
${repeat(
this._uncheckedItems!,
(item) => item.id,
(item) =>
html`
<div class="editRow">
<paper-checkbox
tabindex="0"
?checked=${item.complete}
.itemId=${item.id}
@click=${this._completeItem}
></paper-checkbox>
<paper-input
no-label-float
.value=${item.name}
.itemId=${item.id}
@change=${this._saveEdit}
></paper-input>
${this._reordering
? html`
<div id="sortable">
${guard([this._uncheckedItems, this._renderEmptySortable], () =>
this._renderEmptySortable
? ""
: this._renderItems(this._uncheckedItems!)
)}
</div>
`
)}
: this._renderItems(this._uncheckedItems!)}
${this._checkedItems!.length > 0
? html`
<div class="divider"></div>
Expand All @@ -151,16 +164,16 @@ class HuiShoppingListCard extends SubscribeMixin(LitElement)
"ui.panel.lovelace.cards.shopping-list.checked_items"
)}
</span>
<ha-icon
<ha-svg-icon
class="clearall"
tabindex="0"
icon="hass:notification-clear-all"
.path=${mdiNotificationClearAll}
.title=${this.hass!.localize(
"ui.panel.lovelace.cards.shopping-list.clear_items"
)}
@click=${this._clearItems}
>
</ha-icon>
</ha-svg-icon>
</div>
${repeat(
this._checkedItems!,
Expand Down Expand Up @@ -189,6 +202,44 @@ class HuiShoppingListCard extends SubscribeMixin(LitElement)
`;
}

private _renderItems(items: ShoppingListItem[]) {
return html`
${repeat(
items,
(item) => item.id,
(item) =>
html`
<div class="editRow" item-id=${item.id}>
Comment thread
ShaneQi marked this conversation as resolved.
<paper-checkbox
tabindex="0"
?checked=${item.complete}
.itemId=${item.id}
@click=${this._completeItem}
></paper-checkbox>
<paper-input
no-label-float
.value=${item.name}
.itemId=${item.id}
@change=${this._saveEdit}
></paper-input>
${this._reordering
? html`
<ha-svg-icon
.title=${this.hass!.localize(
"ui.panel.lovelace.cards.shopping-list.drag_and_drop"
)}
class="reorderButton"
.path=${mdiDrag}
>
</ha-svg-icon>
`
: ""}
</div>
`
)}
`;
}

private async _fetchData(): Promise<void> {
if (!this.hass) {
return;
Expand Down Expand Up @@ -250,6 +301,49 @@ class HuiShoppingListCard extends SubscribeMixin(LitElement)
}
}

private async _toggleReorder() {
if (!Sortable) {
const sortableImport = await import(
"sortablejs/modular/sortable.core.esm"
);
Sortable = sortableImport.Sortable;
}
this._reordering = !this._reordering;
await this.updateComplete;
if (this._reordering) {
this._createSortable();
} else {
this._sortable?.destroy();
this._sortable = undefined;
}
}

private _createSortable() {
this._sortable = new Sortable(this.shadowRoot!.getElementById("sortable"), {
Comment thread
ShaneQi marked this conversation as resolved.
Outdated
Comment thread
ShaneQi marked this conversation as resolved.
Outdated
animation: 150,
fallbackClass: "sortable-fallback",
dataIdAttr: "item-id",
handle: "ha-svg-icon",
onSort: async (evt) => {
reorderItems(this.hass!, this._sortable.toArray()).catch(() =>
this._fetchData()

@bramkragten bramkragten Nov 10, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should let the user know it failed and why we undo all the sorting work.

@ShaneQi ShaneQi Nov 27, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will add a toast for the error message tonight.

);
// Move the shopping list item in memory.
this._uncheckedItems!.splice(
evt.newIndex,
0,
this._uncheckedItems!.splice(evt.oldIndex, 1)[0]
);
this._renderEmptySortable = true;

@bramkragten bramkragten Nov 10, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should also do a round of _renderEmptySortable when this._uncheckedItems is changed? What happenes when you add an item while you are sorting the list? And what happens when storing the new order (line 329) failed?

@ShaneQi ShaneQi Nov 17, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested the 2 scenarios without changing anything, both work fine.

When this._uncheckedItems is changed, the whole sortable div content will be re-rendered (at line 150), so everything should be straight.

@bramkragten bramkragten Nov 17, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is not true, Lit uses comments to keep track of things, because with sorting we change the DOM but not the comments Lit loses track of some elements. That's why we clear the DOM and render empty and start over. That can cause duplicate items in the list when not rendering empty.

@ShaneQi ShaneQi Nov 17, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll do more digging tonight.

@ShaneQi ShaneQi Nov 27, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bramkragten I took another look, I still think there is no need to explicitly 'do a round of _renderEmptySortable when this._uncheckedItems is changed'.

Because at line 150: guard([this._uncheckedItems, this._renderEmptySortable]... decides that when either one of the 2 values (_uncheckedItems, _renderEmptySortable) changes, the content of sortable div will be re-rendered (doc).

When add an item while sorting the list, _uncheckedItems is changed therefore the content of sortable div will be re-rendered. Unless you are talking about the scenario that: the user adds an item after the user starts dragging and BEFORE the user puts it down, but I don't see how that is possible.
Also when storing the new order failed, _uncheckedItems is changed therefore the content of sortable div will be re-rendered.

In short: the list is messed up only after drag-n-drop, we do a round of _renderEmptySortable to re-render the list (also clean up the zombie elements). This is enough to keep the list always complying to Lit format.
In any other cases, the change of this._uncheckedItems will always re-render list, no need to use _renderEmptySortable, also there is no zombie elements generated by drag-n-drop.

@ShaneQi ShaneQi Nov 27, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But while I was digging into it, I found another edge case bug:

  1. Drag an item around and put the item back to its original position.
  2. The Lit format is messed up by dragging around.
  3. But the cleanup block (toggling _renderEmptySortable) won't be executed because putting the item back to its original position doesn't trigger onSort event.
  4. Therefore the format won't be cleaned up and causing issues in other user interactions.

The bug:

Kapture 2020-11-27 at 16 00 25

The fix I put in is that instead of observing onSort event, we observe onEnd event, which will be triggered even if an item is dragged and put back to its original position. In the block, we check on evt.oldIndex !== evt.newIndex to decide if a websocket message needs to be sent.

After fix:

Kapture 2020-11-27 at 16 06 30

@bramkragten bramkragten Dec 2, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that re-rendering doesn't clear up the nodes that have no comment anymore.

@ShaneQi ShaneQi Dec 2, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bramkragten now I see what your concern is, and it's a very valid point.

My thinking is: the no-comment-nodes are only generated after drag&drop, so we only need to clear them up after drag&drop. We are already doing that in the sortable's onEnd event. That should guarantee we always clear up the no-comment-nodes.

If you still don't think it's safe, I have no problem adding the clear-up when this._uncheckedItems changes. Let me know.

BTW I need to work on the toast to show errors, I forgot it last time but now I remember.

@ShaneQi ShaneQi Dec 28, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bramkragten Thoughts? Does above make sense to you? Or should I go ahead add the clear-up when this._uncheckedItems changes? I don't mind either way.

await this.updateComplete;
while (this._sortableEl?.lastElementChild) {
this._sortableEl.removeChild(this._sortableEl.lastElementChild);
Comment thread
ShaneQi marked this conversation as resolved.
Outdated
}
this._renderEmptySortable = false;
},
});
}

static get styles(): CSSResult {
return css`
ha-card {
Expand Down Expand Up @@ -280,6 +374,11 @@ class HuiShoppingListCard extends SubscribeMixin(LitElement)
cursor: pointer;
}

.reorderButton {
padding-left: 16px;
cursor: pointer;
}

paper-checkbox {
padding-left: 4px;
padding-right: 20px;
Expand Down
4 changes: 3 additions & 1 deletion src/translations/en.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2242,7 +2242,9 @@
"shopping-list": {
"checked_items": "Checked items",
"clear_items": "Clear checked items",
"add_item": "Add item"
"add_item": "Add item",
"reorder_items": "Reorder items",
"drag_and_drop": "Drag and drop"
},
"picture-elements": {
"hold": "Hold:",
Expand Down