Commit 59e9c4bc authored by rbpotter's avatar rbpotter Committed by Commit Bot

Print Preview: Fix custom margins logic and add tests

Correct Print Preview custom margin logic to match the old UI. The
following rules should be followed:
1) If there is no sticky custom margins value, when custom margins are
selected, they should be initialized to the document margins to avoid
a preview regeneration.
2) If there is a sticky value, custom margins should be initialized to
that sticky value, even if this results in a regeneration of the
preview.
3) If at any point the user changes the orientation or the media size,
the sticky custom margins value is cleared. This happens even if the
media size is changed while the margins are not available, i.e., while
previewing a PDF, because the media size value is sticky.

Bug: 870249
Change-Id: Ibbe5084e34a8b80ec452ec9697184f0c274d2132
Reviewed-on: https://chromium-review.googlesource.com/1161288Reviewed-by: default avatarScott Chen <scottchen@chromium.org>
Reviewed-by: default avatarLei Zhang <thestig@chromium.org>
Commit-Queue: Rebekah Potter <rbpotter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584163}
parent 01f1a282
......@@ -259,6 +259,7 @@ js_library("margin_control_container") {
deps = [
":margin_control",
":settings_behavior",
":state",
"../data:coordinate2d",
"../data:margins",
"../data:measurement_system",
......
......@@ -3,6 +3,7 @@
<link rel="import" href="chrome://resources/html/event_tracker.html">
<link rel="import" href="margin_control.html">
<link rel="import" href="settings_behavior.html">
<link rel="import" href="state.html">
<link rel="import" href="../data/coordinate2d.html">
<link rel="import" href="../data/margins.html">
<link rel="import" href="../data/measurement_system.html">
......
......@@ -2,22 +2,24 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
(function() {
'use strict';
/** @const {number} */
const MINIMUM_DISTANCE = 72; // 1 inch
cr.exportPath('print_preview_new');
/**
* @const {!Map<!print_preview.ticket_items.CustomMarginsOrientation, string>}
*/
const MARGIN_KEY_MAP = new Map([
print_preview_new.MARGIN_KEY_MAP = new Map([
[print_preview.ticket_items.CustomMarginsOrientation.TOP, 'marginTop'],
[print_preview.ticket_items.CustomMarginsOrientation.RIGHT, 'marginRight'],
[print_preview.ticket_items.CustomMarginsOrientation.BOTTOM, 'marginBottom'],
[print_preview.ticket_items.CustomMarginsOrientation.LEFT, 'marginLeft']
]);
(function() {
'use strict';
/** @const {number} */
const MINIMUM_DISTANCE = 72; // 1 inch
Polymer({
is: 'print-preview-margin-control-container',
......@@ -41,6 +43,12 @@ Polymer({
/** @type {?print_preview.MeasurementSystem} */
measurementSystem: Object,
/** @type {!print_preview_new.State} */
state: {
type: Number,
observer: 'onStateChanged_',
},
/** @private {number} */
scaleTransform_: {
type: Number,
......@@ -107,6 +115,8 @@ Polymer({
observers: [
'onMarginSettingsChange_(settings.customMargins.value)',
'onMediaSizeOrLayoutChange_(' +
'settings.mediaSize.value, settings.layout.value)',
],
/** @private {!print_preview.Coordinate2d} */
......@@ -115,6 +125,9 @@ Polymer({
/** @private {?print_preview.Coordinate2d} */
marginStartPositionInPixels_: null,
/** @private {?boolean} */
resetMargins_: null,
/**
* @return {boolean}
* @private
......@@ -128,20 +141,17 @@ Polymer({
/** @private */
onAvailableChange_: function() {
if (this.available_ && !!this.documentMargins) {
if (this.available_ && this.resetMargins_) {
// Set the custom margins values to the current document margins if the
// custom margins were reset.
const newMargins = {};
// Track whether the margins have actually changed to avoid triggering the
// setting change if they are the same.
const oldMargins = this.getSettingValue('customMargins');
let change = false;
for (let side of Object.values(
print_preview.ticket_items.CustomMarginsOrientation)) {
const key = MARGIN_KEY_MAP.get(side);
const key = print_preview_new.MARGIN_KEY_MAP.get(side);
newMargins[key] = this.documentMargins.get(side);
change = change || (newMargins[key] != oldMargins[key]);
}
if (change)
this.setSetting('customMargins', newMargins);
this.setSetting('customMargins', newMargins);
this.resetMargins_ = false;
}
this.invisible_ = !this.available_;
},
......@@ -151,13 +161,44 @@ Polymer({
const margins = this.getSettingValue('customMargins');
this.shadowRoot.querySelectorAll('print-preview-margin-control')
.forEach(control => {
const key = MARGIN_KEY_MAP.get(control.side);
const key = print_preview_new.MARGIN_KEY_MAP.get(control.side);
const newValue = margins[key] || 0;
control.setPositionInPts(newValue);
control.setTextboxValue(this.serializeValueFromPts_(newValue));
});
},
/** @private */
onMediaSizeOrLayoutChange_: function() {
// Reset the custom margins when the paper size changes. Don't do this if it
// is the first preview.
if (this.resetMargins_ === null)
return;
this.resetMargins_ = true;
const marginsSetting = this.getSetting('margins');
if (marginsSetting.value ==
print_preview.ticket_items.MarginsTypeValue.CUSTOM) {
// Set the margins value to default first.
this.setSetting(
'margins', print_preview.ticket_items.MarginsTypeValue.DEFAULT);
}
// Reset custom margins so that the sticky value is not restored for the new
// paper size.
this.setSetting('customMargins', {});
},
/** @private */
onStateChanged_: function() {
if (this.state == print_preview_new.State.READY &&
this.resetMargins_ === null) {
// Don't reset margins if there are sticky values. Otherwise, set them to
// the document margins when the user selects custom margins.
this.resetMargins_ =
this.getSettingValue('customMargins').marginTop === undefined;
}
},
/**
* @param {!print_preview.ticket_items.CustomMarginsOrientation} orientation
* Orientation value to test.
......@@ -298,7 +339,7 @@ Polymer({
side);
const oldMargins = /** @type {print_preview.MarginsSetting} */ (
this.getSettingValue('customMargins'));
const key = MARGIN_KEY_MAP.get(marginSide);
const key = print_preview_new.MARGIN_KEY_MAP.get(marginSide);
if (oldMargins[key] == marginValue)
return;
const newMargins = Object.assign({}, oldMargins);
......
......@@ -152,12 +152,7 @@ Polymer({
key: 'marginsType',
},
customMargins: {
value: {
marginTop: 0,
marginRight: 0,
marginBottom: 0,
marginLeft: 0,
},
value: {},
unavailableValue: {},
valid: true,
available: true,
......
......@@ -154,7 +154,7 @@
<print-preview-margin-control-container id="marginControlContainer"
page-size="[[documentInfo.pageSize]]" settings="{{settings}}"
document-margins="[[documentInfo.margins]]"
measurement-system="[[measurementSystem]]"
measurement-system="[[measurementSystem]]" state="[[state]]"
preview-loaded="[[previewLoaded_]]"
on-margin-drag-changed="onMarginDragChanged_">
</print-preview-margin-control-container>
......
......@@ -574,12 +574,28 @@ Polymer({
onMarginsChanged_: function() {
if (this.getSettingValue('margins') !=
print_preview.ticket_items.MarginsTypeValue.CUSTOM) {
this.lastCustomMargins_ = null;
this.onSettingsChanged_();
} else {
this.lastCustomMargins_ =
const customMargins =
/** @type {!print_preview.MarginsSetting} */ (
this.getSettingValue('customMargins'));
for (let side of Object.values(
print_preview.ticket_items.CustomMarginsOrientation)) {
const key = print_preview_new.MARGIN_KEY_MAP.get(side);
// If custom margins are undefined, return and wait for them to be set.
if (customMargins[key] === undefined || !this.documentInfo ||
!this.documentInfo.margins) {
return;
}
// Start a preview request if the margins actually changed.
if (this.documentInfo.margins.get(side) != customMargins[key]) {
this.onSettingsChanged_();
break;
}
}
this.lastCustomMargins_ = customMargins;
}
},
......@@ -589,6 +605,7 @@ Polymer({
/** @type {!print_preview.MarginsSetting} */ (
this.getSettingValue('customMargins'));
if (!!this.lastCustomMargins_ &&
this.lastCustomMargins_.marginTop !== undefined &&
this.getSettingValue('margins') ==
print_preview.ticket_items.MarginsTypeValue.CUSTOM &&
(this.lastCustomMargins_.marginTop != newValue.marginTop ||
......
......@@ -77,6 +77,7 @@ Polymer({
assertNotReached();
return;
}
this.setSetting(this.settingName, /** @type {Object} */ (newValue));
if (value !== JSON.stringify(this.getSettingValue(this.settingName)))
this.setSetting(this.settingName, /** @type {Object} */ (newValue));
},
});
......@@ -721,6 +721,40 @@ TEST_F('PrintPreviewCustomMarginsTest', 'SetControlsWithTextbox', function() {
this.runMochaTest(custom_margins_test.TestNames.SetControlsWithTextbox);
});
TEST_F(
'PrintPreviewCustomMarginsTest', 'RestoreStickyMarginsAfterDefault',
function() {
this.runMochaTest(
custom_margins_test.TestNames.RestoreStickyMarginsAfterDefault);
});
TEST_F(
'PrintPreviewCustomMarginsTest', 'MediaSizeClearsCustomMargins',
function() {
this.runMochaTest(
custom_margins_test.TestNames.MediaSizeClearsCustomMargins);
});
TEST_F(
'PrintPreviewCustomMarginsTest', 'LayoutClearsCustomMargins', function() {
this.runMochaTest(
custom_margins_test.TestNames.LayoutClearsCustomMargins);
});
TEST_F(
'PrintPreviewCustomMarginsTest', 'IgnoreDocumentMarginsFromPDF',
function() {
this.runMochaTest(
custom_margins_test.TestNames.IgnoreDocumentMarginsFromPDF);
});
TEST_F(
'PrintPreviewCustomMarginsTest', 'MediaSizeClearsCustomMarginsPDF',
function() {
this.runMochaTest(
custom_margins_test.TestNames.MediaSizeClearsCustomMarginsPDF);
});
PrintPreviewNewDestinationSearchTest = class extends NewPrintPreviewTest {
/** @override */
get browsePreload() {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment