Skip to content

Commit

Permalink
fix(time-picker): remove pre-populate value (#1011)
Browse files Browse the repository at this point in the history
* fix(time-picker): remove pre-populate value

* refactor(time-picker): fix typo

* refactor(time-picker): add clarification comment

* test(time-picker): add seconds segment test cases

---------

Co-authored-by: Wattachai Kanawitoon <[email protected]>
  • Loading branch information
Nantawat-Poothong and wattachai-lseg authored Nov 9, 2023
1 parent c746f71 commit 8562ef5
Show file tree
Hide file tree
Showing 2 changed files with 133 additions and 33 deletions.
93 changes: 93 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 @@ -174,6 +174,45 @@ describe('time-picker/TimePicker', function () {
const el = await fixture(timePickerRoleNone);
expect(el).shadowDom.to.equalSnapshot();
});

it('should not pre-populate other segments value when hours value changes', async function () {
const el = await fixture('<ef-time-picker></ef-time-picker>');
const hoursInput = el.hoursInput;
hoursInput.value = '12';
hoursInput.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);
expect(el.seconds).to.equal(null);
});

it('should not pre-populate other segments value when minutes value change', async function () {
const el = await fixture('<ef-time-picker></ef-time-picker>');
const minutesInput = el.minutesInput;
minutesInput.value = '30';
minutesInput.dispatchEvent(
new CustomEvent('focused-changed', { bubbles: true, detail: { value: false } })
);
await elementUpdated(el);
expect(el.hours).to.equal(null);
expect(el.minutes).to.equal(30);
expect(el.seconds).to.equal(null);
});

it('should not pre-populate other segments value when seconds value change', async function () {
const el = await fixture('<ef-time-picker show-seconds></ef-time-picker>');
const secondsInput = el.secondsInput;
secondsInput.value = '45';
secondsInput.dispatchEvent(
new CustomEvent('focused-changed', { bubbles: true, detail: { value: false } })
);
await elementUpdated(el);
expect(el.hours).to.equal(null);
expect(el.minutes).to.equal(null);
expect(el.seconds).to.equal(45);
});
});

describe('Defaults', function () {
Expand Down Expand Up @@ -613,5 +652,59 @@ describe('time-picker/TimePicker', function () {
expect(el.minutes).to.equal(0);
expect(el.hours).to.equal(0);
});

it('Cycling through seconds should not pre-populate other segments value', async function () {
el = await fixture('<ef-time-picker show-seconds></ef-time-picker>');
expect(el.hours).to.equal(null);
expect(el.minutes).to.equal(null);
expect(el.seconds).to.equal(null);
createKeyboardEvent(el.secondsInput, InputKey.arrowDown);
await elementUpdated(el);
expect(el.hours).to.equal(null);
expect(el.minutes).to.equal(null);
expect(el.seconds).to.equal(0);

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

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

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

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

createKeyboardEvent(el.hoursInput, InputKey.arrowDown);
await elementUpdated(el);
expect(el.hours).to.equal(23);
expect(el.minutes).to.equal(null);
expect(el.seconds).to.equal(null);
});
});
});
73 changes: 40 additions & 33 deletions packages/elements/src/time-picker/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import type { FocusedChangedEvent, ValueChangedEvent } from '../events';
import type { NumberField } from '../number-field';

import {
CSSResultGroup,
ControlElement,
Expand All @@ -20,6 +23,8 @@ import {
MILLISECONDS_IN_HOUR,
MILLISECONDS_IN_MINUTE,
MILLISECONDS_IN_SECOND,
MINUTES_IN_HOUR,
SECONDS_IN_MINUTE,
TimeFormat,
addOffset,
format,
Expand All @@ -32,8 +37,6 @@ import {
toTimeSegment
} from '@refinitiv-ui/utils/date.js';

import type { FocusedChangedEvent, ValueChangedEvent } from '../events';
import type { NumberField } from '../number-field';
import '../number-field/index.js';
import { VERSION } from '../version.js';

Expand All @@ -50,6 +53,21 @@ const MAX_SECONDS = 59;
const HOURS_IN_DAY = 24;
const HOURS_OF_NOON = 12;

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

const Placeholder = {
HOURS: '--',
MINUTES: '--',
Expand Down Expand Up @@ -231,7 +249,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 @@ -318,6 +336,13 @@ export class TimePicker extends ControlElement {
return this.showSeconds || this.valueWithSeconds;
}

/**
* True if time value is complete, that is having all the required time segment
*/
private get isCompleteValue(): boolean {
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 +489,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 +733,24 @@ 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 { milliseconds, cycle } = SegmentMap[segment];

if (this.isCompleteValue) {
const offset = segmentValue === null ? 0 : amount * milliseconds;
const value = addOffset(this.currentTimeString, offset);
this.setValueAndNotify(value);
this.selectedSegment = segment;
} else {
// a segment cycle is added to support wrapping of amount with negative value
this[segment] = segmentValue === null ? 0 : (segmentValue + amount + cycle) % cycle;
}

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

/**
Expand Down

0 comments on commit 8562ef5

Please sign in to comment.