Move chunks - UX improvements
- Surround move labels with chip.
- Add move-item icon.
- Remove deprecated dueToMove.
Change-Id: If8d67b9609bee550ebe554bbe32e4fa6e7a425c8
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified_test.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified_test.js
index 24d3635..c08764e 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified_test.js
@@ -116,7 +116,7 @@
assert.equal(rowEls.length, 3);
assert.isTrue(moveControlsRow.classList.contains('movedOut'));
assert.equal(cells.length, 3);
- assert.isTrue(cells[2].classList.contains('moveDescription'));
+ assert.isTrue(cells[2].classList.contains('moveLabel'));
assert.equal(cells[2].textContent, 'Moved out');
});
@@ -139,7 +139,7 @@
assert.equal(rowEls.length, 3);
assert.isTrue(moveControlsRow.classList.contains('movedIn'));
assert.equal(cells.length, 3);
- assert.isTrue(cells[2].classList.contains('moveDescription'));
+ assert.isTrue(cells[2].classList.contains('moveLabel'));
assert.equal(cells[2].textContent, 'Moved in');
});
});
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts
index 75f95f3..f628c47 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts
@@ -927,13 +927,20 @@
controlsClass = 'movedOut';
descriptionIndex = movedOutIndex;
}
- const controls = document.createElement('tr');
+
+ const controls = this._createElement('tr', `moveControls ${controlsClass}`);
const cells = [...Array(numberOfCells).keys()].map(() =>
- document.createElement('td')
+ this._createElement('td')
);
- controls.classList.add('moveControls', controlsClass);
- cells[descriptionIndex].classList.add('moveDescription');
- cells[descriptionIndex].textContent = descriptionText;
+ const moveDescriptionDiv = this._createElement('div', 'moveDescription');
+ const icon = this._createElement('iron-icon');
+ icon.setAttribute('icon', 'gr-icons:move-item');
+ const span = this._createElement('span', '');
+ span.textContent = descriptionText;
+ moveDescriptionDiv.appendChild(icon);
+ moveDescriptionDiv.appendChild(span);
+ cells[descriptionIndex].appendChild(moveDescriptionDiv);
+ cells[descriptionIndex].classList.add('moveLabel');
cells.forEach(c => {
controls.appendChild(c);
});
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.js b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.js
index 718e11b..987a7d0 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.js
@@ -226,8 +226,7 @@
assert.equal(cursorElement.side, 'left');
});
- // To be removed as soon due_to_move (deprecated) is removed
- suite('moved chunks (dueToMove=true)', () => {
+ suite('moved chunks without line range)', () => {
setup(done => {
const renderHandler = function() {
diffElement.removeEventListener('render', renderHandler);
@@ -247,7 +246,7 @@
'Sagittis tincidunt torquent, tempor nunc amet.',
'At rhoncus id.',
],
- due_to_move: true,
+ move_details: {changed: false},
},
{
ab: [
@@ -260,7 +259,7 @@
'Sagittis tincidunt torquent, tempor nunc amet.',
'At rhoncus id.',
],
- due_to_move: true,
+ move_details: {changed: false},
},
{
ab: [
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.ts b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.ts
index 77b5499..c3d758c 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.ts
@@ -366,8 +366,7 @@
const group = new GrDiffGroup(type, lines);
group.keyLocation = !!chunk.keyLocation;
group.dueToRebase = !!chunk.due_to_rebase;
- group.moveDetails =
- chunk.move_details || (chunk.due_to_move ? {changed: false} : undefined);
+ group.moveDetails = chunk.move_details;
group.skip = chunk.skip;
group.ignoredWhitespaceOnly = !!chunk.common;
if (chunk.skip) {
@@ -698,9 +697,6 @@
if (chunk.due_to_rebase) {
subChunk.due_to_rebase = true;
}
- if (chunk.due_to_move) {
- subChunk.due_to_move = true;
- }
if (chunk.move_details) {
subChunk.move_details = chunk.move_details;
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor_test.js b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor_test.js
index ce7a3c4..5496b62 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor_test.js
@@ -1019,16 +1019,6 @@
}
});
- test('_breakdownChunk keeps due_to_move for broken down additions',
- () => {
- sinon.spy(element, '_breakdown');
- const chunk = {b: ['blah', 'blah', 'blah'], due_to_move: true};
- const result = element._breakdownChunk(chunk);
- for (const subResult of result) {
- assert.isTrue(subResult.due_to_move);
- }
- });
-
test('_breakdown common case', () => {
const array = 'Lorem ipsum dolor sit amet, suspendisse inceptos'
.split(' ');
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
index f64d940..ae3e016 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
@@ -16,6 +16,7 @@
*/
import '../../../styles/shared-styles';
import '../../shared/gr-button/gr-button';
+import '../../shared/gr-icons/gr-icons';
import '../gr-diff-builder/gr-diff-builder-element';
import '../gr-diff-highlight/gr-diff-highlight';
import '../gr-diff-selection/gr-diff-selection';
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts
index ee5e1c0..b9578e8 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts
@@ -211,18 +211,45 @@
/* dueToMove */
.dueToMove .content.add .contentText,
- .dueToMove .moveControls.movedIn .moveDescription,
+ .dueToMove .moveControls.movedIn .moveLabel,
.delta.total.dueToMove .content.add .contentText {
- background-color: var(--light-moved-add-highlight-color);
+ background-color: var(--diff-moved-in-background);
}
+
.dueToMove .content.remove .contentText,
- .dueToMove .moveControls.movedOut .moveDescription,
+ .dueToMove .moveControls.movedOut .moveLabel,
.delta.total.dueToMove .content.remove .contentText {
- background-color: var(--light-remove-add-highlight-color);
+ background-color: var(--diff-moved-out-background);
}
- .moveControls {
- text-align: right;
- font-style: italic;
+
+ .delta.dueToMove .movedIn .moveDescription {
+ color: var(--diff-moved-in-background);
+ background-color: var(--diff-moved-in-label-background);
+ }
+ .delta.dueToMove .movedOut .moveDescription {
+ color: var(--diff-moved-out-background);
+ background-color: var(--diff-moved-out-label-background);
+ }
+ .moveLabel {
+ display: flex;
+ justify-content: flex-end;
+ font-family: var(--header-font-family);
+ }
+ .delta.dueToMove .moveDescription {
+ border-radius: var(--spacing-l);
+ padding: var(--spacing-s) var(--spacing-m);
+ margin: var(--spacing-s);
+ font-weight: 500;
+ line-height: var(--spacing-xl);
+ vertical-align: middle;
+ display: flex;
+ }
+
+ .moveDescription iron-icon {
+ color: inherit;
+ margin-right: var(--spacing-s);
+ height: var(--spacing-xl);
+ width: var(--spacing-xl);
}
/* ignoredWhitespaceOnly */
@@ -284,8 +311,8 @@
background-color: var(--view-background-color);
}
.contextBackground {
- /*
- * One line of background behind the context expanders which they can
+ /*
+ * One line of background behind the context expanders which they can
* render on top of, plus some padding.
*/
height: calc(var(--line-height-normal) + var(--spacing-s));
