Skip to content

Commit

Permalink
feat: identify dashboard items by id (#7869)
Browse files Browse the repository at this point in the history
  • Loading branch information
tomivirkki authored Sep 26, 2024
1 parent d325011 commit c272a0d
Show file tree
Hide file tree
Showing 7 changed files with 67 additions and 11 deletions.
18 changes: 17 additions & 1 deletion packages/dashboard/src/vaadin-dashboard-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,22 @@ export const WRAPPER_LOCAL_NAME = 'vaadin-dashboard-widget-wrapper';
// Wrapper proeprties that should be synchronized as widget/section attributes
export const SYNCHRONIZED_ATTRIBUTES = ['editable', 'dragging', 'first-child', 'last-child'];

/**
* Returns true if the given items are equal by reference or by id.
*
* @param {Object} a the first item
* @param {Object} b the second item
*/
export function itemsEqual(a, b) {
if (a === b) {
return true;
}
if (a.id !== undefined && b.id !== undefined) {
return a.id === b.id;
}
return false;
}

/**
* Returns the array of items that contains the given item.
* Might be the dashboard items or the items of a section.
Expand All @@ -18,7 +34,7 @@ export const SYNCHRONIZED_ATTRIBUTES = ['editable', 'dragging', 'first-child', '
* @return {Object[]} the items array
*/
export function getItemsArrayOfItem(item, items) {
if (items.includes(item)) {
if (items.some((i) => itemsEqual(i, item))) {
return items;
}
const parentItem = items.find((i) => i.items && getItemsArrayOfItem(item, i.items));
Expand Down
16 changes: 16 additions & 0 deletions packages/dashboard/src/vaadin-dashboard.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@ import { ElementMixin } from '@vaadin/component-base/src/element-mixin.js';
import { DashboardLayoutMixin } from './vaadin-dashboard-layout-mixin.js';

export interface DashboardItem {
/**
* The id of the item.
* The identifier should be unique among the dashboard items.
* If a unique identifier is not provided, reassigning new item instances
* to the dashboard while a widget is focused may cause the focus to be lost.
*/
id?: unknown;

/**
* The column span of the item
*/
Expand All @@ -26,6 +34,14 @@ export interface DashboardItem {
}

export interface DashboardSectionItem<TItem extends DashboardItem> {
/**
* The id of the item.
* The identifier should be unique among the dashboard items.
* If a unique identifier is not provided, reassigning new item instances
* to the dashboard while a widget is focused may cause the focus to be lost.
*/
id?: unknown;

/**
* The title of the section
*/
Expand Down
13 changes: 8 additions & 5 deletions packages/dashboard/src/vaadin-dashboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { css, ThemableMixin } from '@vaadin/vaadin-themable-mixin/vaadin-themabl
import {
getElementItem,
getItemsArrayOfItem,
itemsEqual,
SYNCHRONIZED_ATTRIBUTES,
WRAPPER_LOCAL_NAME,
} from './vaadin-dashboard-helpers.js';
Expand Down Expand Up @@ -171,13 +172,14 @@ class Dashboard extends ControllerMixin(DashboardLayoutMixin(ElementMixin(Themab
if (wrapper.firstElementChild && wrapper.firstElementChild.localName === 'vaadin-dashboard-section') {
return;
}
if (wrapper.__item.component instanceof HTMLElement) {
if (wrapper.__item.component.parentElement !== wrapper) {
const item = getElementItem(wrapper);
if (item.component instanceof HTMLElement) {
if (item.component.parentElement !== wrapper) {
wrapper.textContent = '';
wrapper.appendChild(wrapper.__item.component);
wrapper.appendChild(item.component);
}
} else if (renderer) {
renderer(wrapper, this, { item: wrapper.__item });
renderer(wrapper, this, { item });
} else {
wrapper.innerHTML = '';
}
Expand All @@ -204,7 +206,7 @@ class Dashboard extends ControllerMixin(DashboardLayoutMixin(ElementMixin(Themab

items.forEach((item) => {
// Find the wrapper for the item or create a new one
const wrapper = wrappers.find((el) => el.__item === item) || this.__createWrapper(item);
const wrapper = wrappers.find((el) => itemsEqual(getElementItem(el), item)) || this.__createWrapper(item);
wrappers = wrappers.filter((el) => el !== wrapper);

// Update the wrapper style
Expand Down Expand Up @@ -306,6 +308,7 @@ class Dashboard extends ControllerMixin(DashboardLayoutMixin(ElementMixin(Themab
${item.rowspan ? `--vaadin-dashboard-item-rowspan: ${item.rowspan};` : ''}
`.trim();

wrapper.__item = item;
wrapper.setAttribute('style', style);
wrapper.editable = this.editable;
wrapper.dragging = this.__widgetReorderController.draggedItem === item;
Expand Down
7 changes: 5 additions & 2 deletions packages/dashboard/src/widget-reorder-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This program is available under Apache License Version 2.0, available at https://vaadin.com/license/
*/

import { getElementItem, getItemsArrayOfItem, WRAPPER_LOCAL_NAME } from './vaadin-dashboard-helpers.js';
import { getElementItem, getItemsArrayOfItem, itemsEqual, WRAPPER_LOCAL_NAME } from './vaadin-dashboard-helpers.js';

const REORDER_EVENT_TIMEOUT = 200;

Expand Down Expand Up @@ -186,7 +186,10 @@ export class WidgetReorderController {
__getDragContextElements() {
const items = getItemsArrayOfItem(this.draggedItem, this.host.items);
return [...this.host.querySelectorAll(WRAPPER_LOCAL_NAME)]
.filter((wrapper) => items && items.includes(wrapper.__item) && wrapper.firstElementChild)
.filter(
(wrapper) =>
items && items.some((item) => itemsEqual(item, getElementItem(wrapper))) && wrapper.firstElementChild,
)
.map((wrapper) => wrapper.firstElementChild);
}

Expand Down
4 changes: 2 additions & 2 deletions packages/dashboard/src/widget-resize-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import { addListener } from '@vaadin/component-base/src/gestures.js';
import { getElementItem, WRAPPER_LOCAL_NAME } from './vaadin-dashboard-helpers.js';
import { getElementItem, itemsEqual, WRAPPER_LOCAL_NAME } from './vaadin-dashboard-helpers.js';

/**
* A controller to widget resizing inside a dashboard.
Expand Down Expand Up @@ -133,7 +133,7 @@ export class WidgetResizeController {

/** @private */
__getItemWrapper(item) {
return [...this.host.querySelectorAll(WRAPPER_LOCAL_NAME)].find((el) => el.__item === item);
return [...this.host.querySelectorAll(WRAPPER_LOCAL_NAME)].find((el) => itemsEqual(el.__item, item));
}

/** @private */
Expand Down
7 changes: 7 additions & 0 deletions packages/dashboard/test/dashboard.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,13 @@ describe('dashboard', () => {
expect(document.activeElement).to.equal(getElementFromCell(dashboard, 0, 0)!);
});

it('should not lose focus when reassigning new items with same ids', async () => {
getElementFromCell(dashboard, 0, 0)!.focus();
dashboard.items = [{ id: '0' }, { id: '1' }];
await nextFrame();
expect(document.activeElement).to.equal(getElementFromCell(dashboard, 0, 0)!);
});

it('should not lose focus when prepending items', async () => {
getElementFromCell(dashboard, 0, 0)!.focus();
dashboard.items = [{ id: '-1' }, ...dashboard.items];
Expand Down
13 changes: 12 additions & 1 deletion packages/dashboard/test/typings/dashboard.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,21 @@ assertType<Dashboard<TestDashboardItem>>(narrowedDashboard);
assertType<Array<TestDashboardItem | DashboardSectionItem<TestDashboardItem>>>(narrowedDashboard.items);
assertType<DashboardRenderer<TestDashboardItem> | null | undefined>(narrowedDashboard.renderer);
assertType<
| { colspan?: number; rowspan?: number; testProperty: string }
| { colspan?: number; rowspan?: number; id?: unknown; testProperty: string }
| { title?: string | null; items: Array<{ colspan?: number; rowspan?: number; testProperty: string }> }
>(narrowedDashboard.items[0]);

const item = narrowedDashboard.items[0] as TestDashboardItem;
assertType<unknown | undefined>(item.id);
assertType<string>(item.testProperty);
assertType<number | undefined>(item.colspan);
assertType<number | undefined>(item.rowspan);

const sectionItem = narrowedDashboard.items[0] as DashboardSectionItem<TestDashboardItem>;
assertType<unknown | undefined>(sectionItem.id);
assertType<string | null | undefined>(sectionItem.title);
assertType<Array<TestDashboardItem | DashboardSectionItem<TestDashboardItem>>>(sectionItem.items);

narrowedDashboard.addEventListener('dashboard-item-moved', (event) => {
assertType<DashboardItemMovedEvent<TestDashboardItem>>(event);
assertType<TestDashboardItem>(event.detail.item);
Expand Down

0 comments on commit c272a0d

Please sign in to comment.