Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(time-picker): remove pre-populate value #995

Merged
merged 12 commits into from
Nov 9, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions packages/elements/src/time-picker/__test__/time-picker.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,18 @@ describe('time-picker/TimePicker', function () {
await elementUpdated(el);
expect(el.seconds).to.equal(null);
});

it('should not pre-populate minutes value when hours value change', async function () {
wattachai-lseg marked this conversation as resolved.
Show resolved Hide resolved
const el = await fixture('<ef-time-picker></ef-time-picker>');
const hoursPart = el.renderRoot.querySelector('#hours');
hoursPart.value = '12';
hoursPart.dispatchEvent(
new CustomEvent('focused-changed', { bubbles: true, detail: { value: false } })
);
await elementUpdated(el);
expect(el.hours).to.equal(12);
expect(el.minutes).to.equal(null);
});
});

describe('Modes', function () {
Expand Down Expand Up @@ -561,5 +573,35 @@ describe('time-picker/TimePicker', function () {
expect(el.minutes).to.equal(0);
expect(el.hours).to.equal(0);
});

it('Cycling through minutes should not pre-populate hours value', async function () {
wattachai-lseg marked this conversation as resolved.
Show resolved Hide resolved
el = await fixture(timePickerDefaults);
expect(el.minutes).to.equal(null);
expect(el.hours).to.equal(null);
createKeyboardEvent(el.minutesInput, InputKey.arrowDown);
await elementUpdated(el);
expect(el.minutes).to.equal(0);
expect(el.hours).to.equal(null);

createKeyboardEvent(el.minutesInput, InputKey.arrowDown);
await elementUpdated(el);
expect(el.minutes).to.equal(59);
expect(el.hours).to.equal(null);
});

it('Cycling through hours should not pre-populate minutes value', async function () {
el = await fixture(timePickerDefaults);
expect(el.minutes).to.equal(null);
expect(el.hours).to.equal(null);
createKeyboardEvent(el.hoursInput, InputKey.arrowDown);
await elementUpdated(el);
expect(el.minutes).to.equal(null);
expect(el.hours).to.equal(0);

createKeyboardEvent(el.hoursInput, InputKey.arrowDown);
await elementUpdated(el);
expect(el.minutes).to.equal(null);
expect(el.hours).to.equal(23);
});
});
});
67 changes: 36 additions & 31 deletions packages/elements/src/time-picker/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import {
MILLISECONDS_IN_HOUR,
MILLISECONDS_IN_MINUTE,
MILLISECONDS_IN_SECOND,
MINUTES_IN_HOUR,
SECONDS_IN_MINUTE,
TimeFormat,
addOffset,
format,
Expand Down Expand Up @@ -49,6 +51,21 @@ const MAX_SECONDS = 59;
const HOURS_IN_DAY = 24;
const HOURS_OF_NOON = 12;

const SegmentMap = {
[Segment.HOURS]: {
miliseconds: MILLISECONDS_IN_HOUR,
cycle: HOURS_IN_DAY
},
[Segment.MINUTES]: {
miliseconds: MILLISECONDS_IN_MINUTE,
cycle: MINUTES_IN_HOUR
},
[Segment.SECONDS]: {
miliseconds: MILLISECONDS_IN_SECOND,
cycle: SECONDS_IN_MINUTE
}
};

const Placeholder = {
HOURS: '--',
MINUTES: '--',
Expand Down Expand Up @@ -230,7 +247,7 @@ export class TimePicker extends ControlElement {
}
}
public override get value(): string {
if (this.hours === null || this.minutes === null || (this.isShowSeconds && this.seconds === null)) {
if (!this.isCompleteValue) {
return '';
}
return this.currentTimeString;
Expand Down Expand Up @@ -317,6 +334,13 @@ export class TimePicker extends ControlElement {
return this.showSeconds || this.valueWithSeconds;
}

/**
* Invert of incomplete value return true if all segments have value
Nantawat-Poothong marked this conversation as resolved.
Show resolved Hide resolved
*/
private get isCompleteValue(): boolean {
Sakchai-Refinitiv marked this conversation as resolved.
Show resolved Hide resolved
return !(this.hours === null || this.minutes === null || (this.isShowSeconds && this.seconds === null));
}

/**
* Get hours taking into account AM/PM placeholder
*/
Expand Down Expand Up @@ -464,20 +488,6 @@ export class TimePicker extends ControlElement {
// no default
}

// Pre-populate empty segments
if (value !== null) {
if (segment === Segment.HOURS && this.minutes === null) {
this.minutes = 0;
}
if (
this.isShowSeconds &&
this.seconds === null &&
(segment === Segment.HOURS || segment === Segment.MINUTES)
) {
this.seconds = 0;
}
}

// verify value again, as time segment validation
// might fail in setter and previous value returned
if (oldValue !== this.value) {
Expand Down Expand Up @@ -722,28 +732,23 @@ export class TimePicker extends ControlElement {
/**
* Changes a time segment value by a specified amount.
* Also updates parent values when rolling through cycles.
* Incomplete value will update only segment without pre-populate value.
* @param amount Amount to change by
* @param segment Segment id
* @returns {void}
*/
private changeValueBy(amount: number, segment: Segment): void {
let offset = 0;
switch (segment) {
case Segment.HOURS:
offset = this.hours === null ? 0 : amount * MILLISECONDS_IN_HOUR;
break;
case Segment.MINUTES:
offset = this.minutes === null ? 0 : amount * MILLISECONDS_IN_MINUTE;
break;
case Segment.SECONDS:
offset = this.seconds === null ? 0 : amount * MILLISECONDS_IN_SECOND;
break;
// no default
const segmentValue = this[segment];
const { miliseconds, cycle } = SegmentMap[segment];

if (this.isCompleteValue) {
Sakchai-Refinitiv marked this conversation as resolved.
Show resolved Hide resolved
const offset = segmentValue === null ? 0 : amount * miliseconds;
const value = addOffset(this.currentTimeString, offset);
this.setValueAndNotify(value);
this.selectedSegment = segment;
} else {
this[segment] = segmentValue === null ? 0 : (segmentValue + amount + cycle) % cycle;
wattachai-lseg marked this conversation as resolved.
Show resolved Hide resolved
}

const value = addOffset(this.currentTimeString, offset);
this.setValueAndNotify(value);
this.selectedSegment = segment;
}

/**
Expand Down
Loading