Skip to content

Commit 342c30f

Browse files
pascalwilbrinkDennis Labordus
pascalwilbrink
and
Dennis Labordus
authored
feat(menu/compareied): Compare View redesign to make it more clear (openscd#996)
* Feature: Added the possibility to ignore attributes and/or elements when checking for differences between 2 elements. Signed-off-by: Pascal Wilbrink <[email protected]> * Cleanup of code Signed-off-by: Pascal Wilbrink <[email protected]> * Reverted package-lock.json Signed-off-by: Pascal Wilbrink <[email protected]> * Refactored and renamed some variables Signed-off-by: Pascal Wilbrink <[email protected]> * Fixed code review comments Signed-off-by: Pascal Wilbrink <[email protected]> * Fixed translation in test Signed-off-by: Pascal Wilbrink <[email protected]> * 919 Make the compare dialog more clear (WIP) Signed-off-by: Pascal Wilbrink <[email protected]> * 919 Make the compare dialog more clear. 2 compare lists within 1 view Signed-off-by: Pascal Wilbrink <[email protected]> * Fixed test (merge conflict) Signed-off-by: Pascal Wilbrink <[email protected]> * Formatted files Signed-off-by: Pascal Wilbrink <[email protected]> * fixed merge conflict Signed-off-by: Pascal Wilbrink <[email protected]> * fixed merge conflict Signed-off-by: Pascal Wilbrink <[email protected]> * fixed review comments Signed-off-by: Pascal Wilbrink <[email protected]> * fixed review comments Signed-off-by: Pascal Wilbrink <[email protected]> * fixed merge conflicts Signed-off-by: Pascal Wilbrink <[email protected]> Signed-off-by: Pascal Wilbrink <[email protected]> * removed compare.test.snap.dev.js Signed-off-by: Pascal Wilbrink <[email protected]> Signed-off-by: Pascal Wilbrink <[email protected]> * Added tsdoc. formatted files Signed-off-by: Pascal Wilbrink <[email protected]> Signed-off-by: Pascal Wilbrink <[email protected]> * Fixed test Signed-off-by: Pascal Wilbrink <[email protected]> Signed-off-by: Pascal Wilbrink <[email protected]> Signed-off-by: Pascal Wilbrink <[email protected]> Co-authored-by: Dennis Labordus <[email protected]>
1 parent 460c99e commit 342c30f

File tree

11 files changed

+867
-1795
lines changed

11 files changed

+867
-1795
lines changed

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
## testing
1313
/coverage/
14+
/**/dist/*.snap.dev.js
1415

1516
## docs
1617
/doc/

‎src/foundation/compare.ts‎

Lines changed: 131 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,31 @@
11
import { html, TemplateResult } from 'lit-element';
22
import { repeat } from 'lit-html/directives/repeat';
3-
import { get, translate } from 'lit-translate';
3+
import { get } from 'lit-translate';
44

55
import '@material/mwc-list';
66
import '@material/mwc-list/mwc-list-item';
77
import '@material/mwc-icon';
88

99
import { identity } from '../foundation.js';
10-
import { nothing } from 'lit-html';
10+
import { SVGTemplateResult } from 'lit-html';
11+
12+
import { attributeIcon, contentIcon, elementIcon } from '../icons/compare.js';
13+
14+
const diffTypeToIcon: Map<DiffType, SVGTemplateResult> = new Map<
15+
DiffType,
16+
SVGTemplateResult
17+
>();
18+
19+
diffTypeToIcon.set('Attribute', attributeIcon);
20+
diffTypeToIcon.set('Content', contentIcon);
21+
diffTypeToIcon.set('Element', elementIcon);
22+
23+
export type DiffType = 'Element' | 'Attribute' | 'Content';
1124

1225
export type Diff<T> =
13-
| { oldValue: T; newValue: null }
14-
| { oldValue: null; newValue: T }
15-
| { oldValue: T; newValue: T };
26+
| { type: DiffType; oldValue: T; newValue: null }
27+
| { type: DiffType; oldValue: null; newValue: T }
28+
| { type: DiffType; oldValue: T; newValue: T };
1629

1730
/**
1831
* Type to filter out a difference based on `tagName`.`attributeName`
@@ -21,7 +34,7 @@ export type Diff<T> =
2134
*/
2235
export interface DiffFilter<T> {
2336
[selector: string]: DiffFilterSelector<T>;
24-
};
37+
}
2538

2639
interface DiffFilterSelector<T> {
2740
full?: DiffFilterConsumer<T>;
@@ -97,9 +110,8 @@ export function diffSclAttributes(
97110
elementToBeCompared: Element,
98111
elementToCompareAgainst: Element,
99112
filterToIgnore: DiffFilter<Element>,
100-
searchElementToBeCompared: Element,
113+
searchElementToBeCompared: Element
101114
): [string, Diff<string>][] {
102-
103115
const attrDiffs: [string, Diff<string>][] = [];
104116

105117
// First check if there is any text inside the element and there should be no child elements.
@@ -120,7 +132,10 @@ export function diffSclAttributes(
120132
);
121133

122134
if (!shouldFilter) {
123-
attrDiffs.push(['value', { newValue: newText, oldValue: oldText }]);
135+
attrDiffs.push([
136+
'value',
137+
{ type: 'Content', newValue: newText, oldValue: oldText },
138+
]);
124139
}
125140
}
126141

@@ -148,6 +163,7 @@ export function diffSclAttributes(
148163
attrDiffs.push([
149164
name,
150165
<Diff<string>>{
166+
type: 'Attribute',
151167
newValue: elementToBeCompared.getAttribute(name),
152168
oldValue: elementToCompareAgainst.getAttribute(name),
153169
},
@@ -214,7 +230,7 @@ export function diffSclChilds(
214230
getDiffFilterSelector(
215231
newElement,
216232
searchElementToBeCompared,
217-
filterToIgnore
233+
filterToIgnore
218234
)
219235
);
220236
if (!shouldFilter) {
@@ -226,9 +242,17 @@ export function diffSclChilds(
226242

227243
if (oldElement) {
228244
childrenToCompareTo.splice(twinIndex, 1);
229-
childDiffs.push({ newValue: newElement, oldValue: oldElement });
245+
childDiffs.push({
246+
type: 'Element',
247+
newValue: newElement,
248+
oldValue: oldElement,
249+
});
230250
} else {
231-
childDiffs.push({ newValue: newElement, oldValue: null });
251+
childDiffs.push({
252+
type: 'Element',
253+
newValue: newElement,
254+
oldValue: null,
255+
});
232256
}
233257
}
234258
}
@@ -244,7 +268,11 @@ export function diffSclChilds(
244268
)
245269
);
246270
if (!shouldFilter) {
247-
childDiffs.push({ newValue: null, oldValue: oldElement });
271+
childDiffs.push({
272+
type: 'Element',
273+
newValue: null,
274+
oldValue: oldElement,
275+
});
248276
}
249277
}
250278
});
@@ -264,7 +292,7 @@ export function renderDiff(
264292
filterToIgnore: DiffFilter<Element> = {}
265293
): TemplateResult | null {
266294
return renderDiffInternal(
267-
elementToBeCompared,
295+
elementToBeCompared,
268296
elementToCompareAgainst,
269297
filterToIgnore,
270298
elementToBeCompared,
@@ -294,7 +322,7 @@ function renderDiffInternal(
294322
elementToBeCompared,
295323
elementToCompareAgainst,
296324
filterToIgnore,
297-
searchElementToBeCompared,
325+
searchElementToBeCompared
298326
);
299327

300328
// Next check which elements are added, deleted or in both elements.
@@ -318,15 +346,7 @@ function renderDiffInternal(
318346

319347
// These children exist in both old and new element, let's check if there are any difference in the children.
320348
const childToCompareTemplates = childToCompare
321-
.map(diff =>
322-
renderDiffInternal(
323-
diff.newValue!,
324-
diff.oldValue!,
325-
filterToIgnore,
326-
searchElementToBeCompared,
327-
searchElementToCompareAgainst
328-
)
329-
)
349+
.map(diff => renderDiff(diff.newValue!, diff.oldValue!, filterToIgnore))
330350
.filter(result => result !== null);
331351

332352
// If there are difference generate the HTML otherwise just return null.
@@ -335,81 +355,93 @@ function renderDiffInternal(
335355
attrDiffs.length > 0 ||
336356
childAddedOrDeleted.length > 0
337357
) {
338-
return html` ${
339-
attrDiffs.length > 0 || childAddedOrDeleted.length > 0
340-
? html` <mwc-list multi>
341-
${
342-
attrDiffs.length > 0
343-
? html` <mwc-list-item noninteractive ?twoline=${!!idTitle}>
344-
<span class="resultTitle">
345-
${translate('compare.attributes', {
346-
elementName: elementToBeCompared.tagName,
347-
})}
348-
</span>
349-
${
350-
idTitle
351-
? html`<span slot="secondary">${idTitle}</span>`
352-
: nothing
353-
}
354-
</mwc-list-item>
355-
<li padded divider role="separator"></li>`
356-
: ''
357-
}
358-
${repeat(
359-
attrDiffs,
360-
e => e,
361-
([name, diff]) =>
362-
html` <mwc-list-item twoline left hasMeta>
363-
<span>${name}</span>
364-
<span slot="secondary">
365-
${diff.oldValue ?? ''}
366-
${diff.oldValue && diff.newValue ? html`&curarr;` : ' '}
367-
${diff.newValue ?? ''}
368-
</span>
369-
<mwc-icon slot="meta">
370-
${diff.oldValue ? (diff.newValue ? 'edit' : 'delete') : 'add'}
371-
</mwc-icon>
372-
</mwc-list-item>`
373-
)}
374-
${
375-
childAddedOrDeleted.length > 0
376-
? html` <mwc-list-item noninteractive ?twoline=${!!idTitle}>
377-
<span class="resultTitle">
378-
${translate('compare.children', {
379-
elementName: elementToBeCompared.tagName,
380-
})}
381-
</span>
382-
${
383-
idTitle
384-
? html`<span slot="secondary">${idTitle}</span>`
385-
: nothing
386-
}
387-
</mwc-list-item>
388-
<li padded divider role="separator"></li>`
389-
: ''
390-
}
391-
${repeat(
392-
childAddedOrDeleted,
393-
e => e,
394-
diff =>
395-
html` <mwc-list-item twoline left hasMeta>
396-
<span>${diff.oldValue?.tagName ?? diff.newValue?.tagName}</span>
397-
<span slot="secondary">
398-
${
399-
diff.oldValue
400-
? describe(diff.oldValue)
401-
: describe(diff.newValue)
402-
}
403-
</span>
404-
<mwc-icon slot="meta">
405-
${diff.oldValue ? 'delete' : 'add'}
406-
</mwc-icon>
407-
</mwc-list-item>`
408-
)}
409-
</mwc-list>`
410-
: ''
411-
}
358+
return html` ${attrDiffs.length > 0 || childAddedOrDeleted.length > 0
359+
? html`<div class="container container--alt">
360+
<div class="list__container list__container--left">
361+
<mwc-list multi right nonInteractive>
362+
${repeat(
363+
attrDiffs,
364+
e => e,
365+
([name, diff]) =>
366+
html`<mwc-list-item right twoLine graphic="icon">
367+
${diff.oldValue !== null
368+
? html`
369+
<span>
370+
${name}:
371+
${diff.oldValue === '' ? '""' : diff.oldValue}
372+
</span>
373+
<span slot="secondary">${idTitle}</span>
374+
<mwc-icon slot="graphic">
375+
${diffTypeToIcon.get(diff.type)}
376+
</mwc-icon>
377+
`
378+
: ''}
379+
</mwc-list-item>`
380+
)}
381+
${repeat(
382+
childAddedOrDeleted,
383+
e => e,
384+
diff =>
385+
html` <mwc-list-item right twoLine graphic="icon">
386+
${diff.oldValue
387+
? html`
388+
<span>${diff.oldValue.tagName}</span>
389+
<span slot="secondary">
390+
${describe(diff.oldValue!)}
391+
</span>
392+
<mwc-icon slot="graphic">
393+
${diffTypeToIcon.get(diff.type)}
394+
</mwc-icon>
395+
`
396+
: ''}
397+
</mwc-list-item>`
398+
)}
399+
</mwc-list>
400+
</div>
401+
<div class="list__container">
402+
<mwc-list multi left nonInteractive>
403+
${repeat(
404+
attrDiffs,
405+
e => e,
406+
([name, diff]) =>
407+
html` <mwc-list-item left twoLine graphic="icon">
408+
${diff.newValue !== null
409+
? html`
410+
<span>
411+
${name}:
412+
${diff.newValue === '' ? '""' : diff.newValue}
413+
</span>
414+
<span slot="secondary">${idTitle}</span>
415+
<mwc-icon slot="graphic">
416+
${diffTypeToIcon.get(diff.type)}
417+
</mwc-icon>
418+
`
419+
: ''}
420+
</mwc-list-item>`
421+
)}
422+
${repeat(
423+
childAddedOrDeleted,
424+
e => e,
425+
diff =>
426+
html` <mwc-list-item left twoLine graphic="icon">
427+
${diff.newValue
428+
? html`
429+
<span>${diff.newValue.tagName}</span>
430+
<span slot="secondary">
431+
${describe(diff.newValue!)}
432+
</span>
433+
<mwc-icon slot="graphic">
434+
${diffTypeToIcon.get(diff.type)}
435+
</mwc-icon>
436+
`
437+
: ''}
438+
</mwc-list-item>`
439+
)}
440+
</mwc-list>
441+
</div>
442+
</div>`
443+
: ''}
412444
${childToCompareTemplates}`;
413445
}
414446
return null;
415-
}
447+
}

‎src/icons/compare.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import { svg } from "lit-html";
2+
3+
export const elementIcon = svg`<svg style="width:24px;height:24px" viewBox="0 0 24 24">
4+
<path fill="currentColor" d="M9,7H15V9H11V11H15V13H11V15H15V17H9V7M12,2A10,10 0 0,1 22,12A10,10 0 0,1 12,22A10,10 0 0,1 2,12A10,10 0 0,1 12,2M12,4A8,8 0 0,0 4,12A8,8 0 0,0 12,20A8,8 0 0,0 20,12A8,8 0 0,0 12,4Z" />
5+
</svg>`;
6+
7+
export const attributeIcon = svg`<svg viewBox="0 0 24 24">
8+
<path fill="currentColor" d="M11,7H13A2,2 0 0,1 15,9V17H13V13H11V17H9V9A2,2 0 0,1 11,7M11,9V11H13V9H11M12,20A8,8 0 0,0 20,12A8,8 0 0,0 12,4A8,8 0 0,0 4,12A8,8 0 0,0 12,20M12,2A10,10 0 0,1 22,12A10,10 0 0,1 12,22A10,10 0 0,1 2,12A10,10 0 0,1 12,2Z" />
9+
</svg>`;
10+
11+
export const contentIcon = svg`<svg viewBox="0 0 24 24">
12+
<path fill="currentColor" d="M11,7H13A2,2 0 0,1 15,9V10H13V9H11V15H13V14H15V15A2,2 0 0,1 13,17H11A2,2 0 0,1 9,15V9A2,2 0 0,1 11,7M12,2A10,10 0 0,1 22,12A10,10 0 0,1 12,22A10,10 0 0,1 2,12A10,10 0 0,1 12,2M12,4A8,8 0 0,0 4,12A8,8 0 0,0 12,20A8,8 0 0,0 20,12A8,8 0 0,0 12,4Z" />
13+
</svg>`;

0 commit comments

Comments
 (0)