Skip to content

feat: support partial i18n in time picker #9141

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+89 −66
Open

feat: support partial i18n in time picker #9141

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 8 additions & 5 deletions packages/time-picker/src/vaadin-time-picker-mixin.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import type { FocusMixinClass } from '@vaadin/a11y-base/src/focus-mixin.js';
import type { KeyboardMixinClass } from '@vaadin/a11y-base/src/keyboard-mixin.js';
import type { ControllerMixinClass } from '@vaadin/component-base/src/controller-mixin.js';
import type { DelegateStateMixinClass } from '@vaadin/component-base/src/delegate-state-mixin.js';
import type { I18nMixinClass } from '@vaadin/component-base/src/i18n-mixin.js';
import type { SlotStylesMixinClass } from '@vaadin/component-base/src/slot-styles-mixin.js';
import type { ClearButtonMixinClass } from '@vaadin/field-base/src/clear-button-mixin.js';
import type { FieldMixinClass } from '@vaadin/field-base/src/field-mixin.js';
Expand All @@ -22,8 +23,9 @@ import type { ValidateMixinClass } from '@vaadin/field-base/src/validate-mixin.j
import type { TimePickerTime } from './vaadin-time-picker-helper.js';

export interface TimePickerI18n {
parseTime(time: string): TimePickerTime | undefined;
formatTime(time: TimePickerTime | undefined): string;
parseTime?(time: string): TimePickerTime | undefined;

formatTime?(time: TimePickerTime | undefined): string;
}

/**
Expand All @@ -38,6 +40,7 @@ export declare function TimePickerMixin<T extends Constructor<HTMLElement>>(
Constructor<DisabledMixinClass> &
Constructor<FieldMixinClass> &
Constructor<FocusMixinClass> &
Constructor<I18nMixinClass<TimePickerI18n>> &
Constructor<InputConstraintsMixinClass> &
Constructor<InputControlMixinClass> &
Constructor<InputMixinClass> &
Expand Down Expand Up @@ -117,9 +120,9 @@ export declare class TimePickerMixinClass {
overlayClass: string;

/**
* The object used to localize this component.
* To change the default localization, replace the entire
* _i18n_ object or just the property you want to modify.
* The object used to localize this component. To change the default
* localization, replace this with an object that provides all properties, or
* just the individual properties you want to change.
*
* The object has the following JSON structure:
*
Expand Down
106 changes: 55 additions & 51 deletions packages/time-picker/src/vaadin-time-picker-mixin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* Copyright (c) 2018 - 2025 Vaadin Ltd.
* This program is available under Apache License Version 2.0, available at https://vaadin.com/license/
*/
import { I18nMixin } from '@vaadin/component-base/src/i18n-mixin.js';
import { TooltipController } from '@vaadin/component-base/src/tooltip-controller.js';
import { InputControlMixin } from '@vaadin/field-base/src/input-control-mixin.js';
import { InputController } from '@vaadin/field-base/src/input-controller.js';
Expand All @@ -22,11 +23,12 @@ const MAX_ALLOWED_TIME = '23:59:59.999';
* A mixin providing common time-picker functionality.
*
* @polymerMixin
* @mixes I18nMixin
* @mixes InputControlMixin
* @mixes PatternMixin
*/
export const TimePickerMixin = (superClass) =>
class TimePickerMixinClass extends PatternMixin(InputControlMixin(superClass)) {
class TimePickerMixinClass extends I18nMixin(timePickerI18nDefaults, PatternMixin(InputControlMixin(superClass))) {
static get properties() {
return {
/**
Expand Down Expand Up @@ -125,43 +127,6 @@ export const TimePickerMixin = (superClass) =>
type: String,
},

/**
* The object used to localize this component.
* To change the default localization, replace the entire
* _i18n_ object or just the property you want to modify.
*
* The object has the following JSON structure:
*
* ```
* {
* // A function to format given `Object` as
* // time string. Object is in the format `{ hours: ..., minutes: ..., seconds: ..., milliseconds: ... }`
* formatTime: (time) => {
* // returns a string representation of the given
* // object in `hh` / 'hh:mm' / 'hh:mm:ss' / 'hh:mm:ss.fff' - formats
* },
*
* // A function to parse the given text to an `Object` in the format
* // `{ hours: ..., minutes: ..., seconds: ..., milliseconds: ... }`.
* // Must properly parse (at least) text
* // formatted by `formatTime`.
* parseTime: text => {
* // Parses a string in object/string that can be formatted by`formatTime`.
* }
* }
* ```
*
* Both `formatTime` and `parseTime` need to be implemented
* to ensure the component works properly.
*
* @type {!TimePickerI18n}
*/
i18n: {
type: Object,
sync: true,
value: () => ({ ...timePickerI18nDefaults }),
},

/** @private */
_comboBoxValue: {
type: String,
Expand All @@ -185,7 +150,7 @@ export const TimePickerMixin = (superClass) =>
static get observers() {
return [
'__updateAriaAttributes(__dropdownItems, opened, inputElement)',
'__updateDropdownItems(i18n, min, max, step)',
'__updateDropdownItems(__effectiveI18n, min, max, step)',
];
}

Expand All @@ -202,14 +167,53 @@ export const TimePickerMixin = (superClass) =>
return this.$.clearButton;
}

/**
* The object used to localize this component. To change the default
* localization, replace this with an object that provides all properties, or
* just the individual properties you want to change.
*
* The object has the following JSON structure:
*
* ```
* {
* // A function to format given `Object` as
* // time string. Object is in the format `{ hours: ..., minutes: ..., seconds: ..., milliseconds: ... }`
* formatTime: (time) => {
* // returns a string representation of the given
* // object in `hh` / 'hh:mm' / 'hh:mm:ss' / 'hh:mm:ss.fff' - formats
* },
*
* // A function to parse the given text to an `Object` in the format
* // `{ hours: ..., minutes: ..., seconds: ..., milliseconds: ... }`.
* // Must properly parse (at least) text
* // formatted by `formatTime`.
* parseTime: text => {
* // Parses a string in object/string that can be formatted by`formatTime`.
* }
* }
* ```
*
* Both `formatTime` and `parseTime` need to be implemented
* to ensure the component works properly.
Comment on lines +196 to +197
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think this statement can be removed with the partial i18n approach, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That part is indeed a bit confusing at the moment. Technically you can omit either function, but then the component likely won't work correctly, as the component logic depends on the parse function to work with what the format function produces. But I didn't have any good ideas on how to make that clearer. I can take another look if I can formulate this better.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. We can keep probably keep it, and consider this change a refactoring.

*
* @return {!TimePickerI18n}
*/
get i18n() {
return super.i18n;
}

set i18n(value) {
super.i18n = value;
}

/**
* The input element's value when it cannot be parsed as a time, and an empty string otherwise.
*
* @private
* @return {string}
*/
get __unparsableValue() {
if (this._inputElementValue && !this.i18n.parseTime(this._inputElementValue)) {
if (this._inputElementValue && !this.__effectiveI18n.parseTime(this._inputElementValue)) {
return this._inputElementValue;
}

Expand Down Expand Up @@ -285,8 +289,8 @@ export const TimePickerMixin = (superClass) =>
checkValidity() {
return !!(
this.inputElement.checkValidity() &&
(!this.value || this._timeAllowed(this.i18n.parseTime(this.value))) &&
(!this._comboBoxValue || this.i18n.parseTime(this._comboBoxValue))
(!this.value || this._timeAllowed(this.__effectiveI18n.parseTime(this.value))) &&
(!this._comboBoxValue || this.__effectiveI18n.parseTime(this._comboBoxValue))
);
}

Expand Down Expand Up @@ -354,7 +358,7 @@ export const TimePickerMixin = (superClass) =>
// observer where the value can be parsed again, so we set
// this flag to ensure it does not alter the value.
this.__useMemo = true;
this._comboBoxValue = this.i18n.formatTime(objWithStep);
this._comboBoxValue = this.__effectiveI18n.formatTime(objWithStep);
this.__useMemo = false;

this.__commitValueChange();
Expand Down Expand Up @@ -451,7 +455,7 @@ export const TimePickerMixin = (superClass) =>
}

/** @private */
__updateDropdownItems(i18n, min, max, step) {
__updateDropdownItems(effectiveI18n, min, max, step) {
const minTimeObj = validateTime(parseISOTime(min || MIN_ALLOWED_TIME), step);
const minSec = this.__getSec(minTimeObj);

Expand All @@ -467,7 +471,7 @@ export const TimePickerMixin = (superClass) =>
}

if (this.value) {
this._comboBoxValue = i18n.formatTime(i18n.parseTime(this.value));
this._comboBoxValue = effectiveI18n.formatTime(effectiveI18n.parseTime(this.value));
}
}

Expand Down Expand Up @@ -503,7 +507,7 @@ export const TimePickerMixin = (superClass) =>
while (time + step >= minSec && time + step <= maxSec) {
const timeObj = validateTime(this.__addStep(time * 1000, step), step);
time += step;
const formatted = this.i18n.formatTime(timeObj);
const formatted = this.__effectiveI18n.formatTime(timeObj);
generatedList.push({ label: formatted, value: formatted });
}

Expand Down Expand Up @@ -549,8 +553,8 @@ export const TimePickerMixin = (superClass) =>
return;
}

const parsedObj = this.__useMemo ? this.__memoValue : this.i18n.parseTime(value);
const newValue = this.i18n.formatTime(parsedObj) || '';
const parsedObj = this.__useMemo ? this.__memoValue : this.__effectiveI18n.parseTime(value);
const newValue = this.__effectiveI18n.formatTime(parsedObj) || '';

if (parsedObj) {
if (value !== newValue) {
Expand Down Expand Up @@ -593,7 +597,7 @@ export const TimePickerMixin = (superClass) =>

/** @private */
__updateInputValue(obj) {
const timeString = this.i18n.formatTime(validateTime(obj, this.step)) || '';
const timeString = this.__effectiveI18n.formatTime(validateTime(obj, this.step)) || '';
this._comboBoxValue = timeString;
}

Expand All @@ -605,8 +609,8 @@ export const TimePickerMixin = (superClass) =>
* @protected
*/
_timeAllowed(time) {
const parsedMin = this.i18n.parseTime(this.min || MIN_ALLOWED_TIME);
const parsedMax = this.i18n.parseTime(this.max || MAX_ALLOWED_TIME);
const parsedMin = this.__effectiveI18n.parseTime(this.min || MIN_ALLOWED_TIME);
const parsedMax = this.__effectiveI18n.parseTime(this.max || MAX_ALLOWED_TIME);

return (
(!this.__getMsec(parsedMin) || this.__getMsec(time) >= this.__getMsec(parsedMin)) &&
Expand Down
20 changes: 11 additions & 9 deletions packages/time-picker/test/keyboard-navigation.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,15 +109,17 @@ describe('keyboard navigation', () => {

describe('with custom parser and formatter', () => {
beforeEach(() => {
timePicker.i18n.parseTime = (text) => {
const parts = text.split('.');
return {
hours: parts[0],
minutes: parts[1],
};
};
timePicker.i18n.formatTime = (time) => {
return `${time.hours}.${time.minutes}`;
timePicker.i18n = {
parseTime(text) {
const parts = text.split('.');
return {
hours: parts[0],
minutes: parts[1],
};
},
formatTime(time) {
return `${time.hours}.${time.minutes}`;
},
};
});

Expand Down
10 changes: 9 additions & 1 deletion packages/time-picker/test/time-picker.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ describe('time-picker', () => {

describe('custom functions', () => {
it('should use custom parser if that exists', () => {
timePicker.i18n = { ...timePicker.i18n, parseTime: sinon.stub().returns({ hours: 12, minutes: 0, seconds: 0 }) };
timePicker.i18n = { parseTime: sinon.stub().returns({ hours: 12, minutes: 0, seconds: 0 }) };
timePicker.value = '12';
expect(timePicker.i18n.parseTime.args[0][0]).to.be.equal('12:00');
expect(timePicker.value).to.be.equal('12:00');
Expand Down Expand Up @@ -433,6 +433,14 @@ describe('time-picker', () => {
expect(inputElement.value).to.equal('1200');
expect(timePicker.value).to.equal('12:00');
});

it('should fallback to default functions if none are provided', () => {
timePicker.i18n = {};

timePicker.value = '12:00';
expect(inputElement.value).to.equal('12:00');
expect(timePicker.value).to.equal('12:00');
});
});

describe('helper text', () => {
Expand Down
6 changes: 6 additions & 0 deletions packages/time-picker/test/typings/time-picker.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type {
import type { ControllerMixinClass } from '@vaadin/component-base/src/controller-mixin.js';
import type { DirMixinClass } from '@vaadin/component-base/src/dir-mixin.js';
import type { ElementMixinClass } from '@vaadin/component-base/src/element-mixin.js';
import type { I18nMixinClass } from '@vaadin/component-base/src/i18n-mixin.js';
import type { ClearButtonMixinClass } from '@vaadin/field-base/src/clear-button-mixin.js';
import type { InputControlMixinClass } from '@vaadin/field-base/src/input-control-mixin.js';
import type { PatternMixinClass } from '@vaadin/field-base/src/pattern-mixin.js';
Expand All @@ -15,6 +16,7 @@ import type { TimePickerItem } from '../../src/vaadin-time-picker-item.js';
import type {
TimePicker,
TimePickerChangeEvent,
TimePickerI18n,
TimePickerInvalidChangedEvent,
TimePickerOpenedChangedEvent,
TimePickerValidatedEvent,
Expand All @@ -28,6 +30,7 @@ const timePicker = document.createElement('vaadin-time-picker');
// Mixins
assertType<ControllerMixinClass>(timePicker);
assertType<ElementMixinClass>(timePicker);
assertType<I18nMixinClass<TimePickerI18n>>(timePicker);
assertType<InputControlMixinClass>(timePicker);
assertType<ClearButtonMixinClass>(timePicker);
assertType<PatternMixinClass>(timePicker);
Expand Down Expand Up @@ -69,6 +72,9 @@ timePicker.addEventListener('validated', (event) => {
assertType<boolean>(event.detail.valid);
});

// I18n
assertType<TimePickerI18n>({});

// Item
const item = document.createElement('vaadin-time-picker-item');
assertType<TimePickerItem>(item);
Expand Down