Skip to content

Commit

Permalink
fix(ui5-timepicker): fix firing "change" event for the same value (#1764
Browse files Browse the repository at this point in the history
)

Previously the TImePicker used to fire "change" event unconditionally upon "Submit" press. 
Now, the value is checked and if it is changed, the event will be fired.
  • Loading branch information
ilhan007 authored Jun 10, 2020
1 parent 377075a commit 3a0c7d5
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 7 deletions.
15 changes: 12 additions & 3 deletions packages/main/src/TimePicker.js
Original file line number Diff line number Diff line change
Expand Up @@ -281,8 +281,7 @@ class TimePicker extends UI5Element {
constructor() {
super();

this.readonly = false;
this.disabled = false;
this.prevValue = null;
this._isPickerOpen = false;
this.i18nBundle = getI18nBundle("@ui5/webcomponents");

Expand Down Expand Up @@ -531,9 +530,13 @@ class TimePicker extends UI5Element {
selectedDate.setMinutes(minutes);
selectedDate.setSeconds(seconds);

this.setPrevValue(this.value);
this.setValue(this.getFormat().format(selectedDate));

this.fireEvent("change", { value: this.value, valid: true });
if (this.prevValue !== this.value) {
this.fireEvent("change", { value: this.value, valid: true });
this.previousValue = this.value;
}

this.closePicker();
}
Expand Down Expand Up @@ -700,6 +703,12 @@ class TimePicker extends UI5Element {
}
}

setPrevValue(value) {
if (this.isValid(value)) {
this.prevValue = this.normalizeValue(value);
}
}

/**
* Formats a Java Script date object into a string representing a locale date and time
* according to the <code>formatPattern</code> property of the TimePicker instance
Expand Down
8 changes: 4 additions & 4 deletions packages/main/test/pages/TimePicker.html
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@

<br /><br />
<ui5-title>Test "change" event</ui5-title>
<ui5-timepicker id="timepickerChange"></ui5-timepicker>
<ui5-input id="changeResult"></ui5-input>
<ui5-timepicker id="timepickerChange" format-pattern="HH:mm" value="12:00"></ui5-timepicker>
<ui5-input id="changeResult" value="0"></ui5-input>

<br /><br />
<ui5-title>Test "input" event</ui5-title>
Expand All @@ -57,12 +57,12 @@

<script>
var counter = 0;
timepickerChange.addEventListener("change", function() {
timepickerChange.addEventListener("ui5-change", function() {
changeResult.value = ++counter;
});

var inputCounter = 0;
timepickerInput.addEventListener("input", function() {
timepickerInput.addEventListener("ui5-input", function() {
inputResult.value = ++inputCounter;
});

Expand Down
38 changes: 38 additions & 0 deletions packages/main/test/specs/TimePicker.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,44 @@ describe("TimePicker general interaction", () => {
assert.notOk(slot.error, "cValue State message slot is working");
});

it("tests change event", () => {
const staticAreaItemClassName = browser.getStaticAreaItemClassName("#timepickerChange");
const timepicker = browser.$("#timepickerChange");
const icon = timepicker.shadow$("ui5-input").$("ui5-icon");
const changeResult = browser.$("#changeResult");

// act - submit the same time
icon.click();
const timepickerPopover = browser.$(`.${staticAreaItemClassName}`).shadow$("ui5-responsive-popover");
timepickerPopover.$("#submit").click();

// assert
assert.strictEqual(changeResult.getProperty("value"), "0", "Change not fired as expected");

// act - submit value after changing time
icon.click();
timepickerPopover.$(".ui5-timepicker-hours-wheelslider").setProperty("value", "10");
timepickerPopover.$("#submit").click();

// assert
assert.strictEqual(changeResult.getProperty("value"), "1", "Change fired as expected");

// act - submit the same time
icon.click();
timepickerPopover.$("#submit").click();

// assert
assert.strictEqual(changeResult.getProperty("value"), "1", "Change not fired as expected");

// act - submit value after changing time
icon.click();
timepickerPopover.$(".ui5-timepicker-hours-wheelslider").setProperty("value", "11");
timepickerPopover.$("#submit").click();

// assert
assert.strictEqual(changeResult.getProperty("value"), "2", "Change fired as expected");
});

it("tests value state", () => {
const timepicker = browser.$("#timepickerEmptyValue");
const button = browser.$("#testBtn");
Expand Down

0 comments on commit 3a0c7d5

Please sign in to comment.