-
Notifications
You must be signed in to change notification settings - Fork 60
[UIK-3464][time-picker] rewrote component to ts/refactoring time picker component/added unit tests for core time picker elements #2641
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
Changes from 35 commits
2190694
8fea293
da43009
829f8ba
a2ee282
df8bbce
9d403cd
37f1e68
1357916
a15eb8c
efc2f96
f0715f3
647997e
21779f9
6c80b3f
d4babde
c3af396
24d9d07
822c5fe
e80a828
2fc4135
11d7394
199b49f
3e555a6
6630411
3dc1d2d
18b5dd4
dd31537
633823c
1b976c4
63df0d2
ad658b0
442740d
9889e54
ba7b78f
3fb65a6
52d9a82
ceebe7d
6166d2e
e19f026
dd888b9
ec1a4dc
379bdc7
5e09c3d
796f0ac
bb840ae
68e557f
4e8fb23
f493284
b1bba2d
e021aea
088384b
f64527c
ac769c4
19cb97d
df95110
ef6ee30
5ba7211
54e7654
c0f8488
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| type Primitive = string | number | boolean | symbol | bigint | null | undefined; | ||
| type IsReadonly<This, Property extends keyof This> = | ||
| (<F>() => F extends { [P in Property]: This[Property] } ? 1 : 2) extends | ||
| (<F>() => F extends { -readonly [P in Property]: This[Property] } ? 1 : 2) | ||
| ? false | ||
| : true; | ||
| type Callback<This> = (this: This, field: string | symbol, newValue: any) => void; | ||
| type ReturnType< | ||
| This, | ||
| Property extends keyof This = keyof This, | ||
| > = IsReadonly<This, Property> extends true | ||
| ? (_: undefined, ctx: ClassFieldDecoratorContext<This, This[Property]>) => void | ||
| : This[Property] extends Primitive | ||
| ? (_: undefined, ctx: ClassFieldDecoratorContext<This, This[Property]>) => void | ||
| : never; | ||
|
|
||
| const isPrimitiveValue = (value: any) => value !== Object(value); | ||
ilyabrower marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| function reactive<This>(cb: Callback<This>): ReturnType<This>; | ||
| function reactive< | ||
| This, | ||
| Property extends keyof This, | ||
| Value = This[Property], | ||
| >(watchedFields: Value extends Primitive ? never : Array<keyof Value>, cb: Callback<This>): ReturnType<This>; | ||
|
|
||
| function reactive< | ||
| This, | ||
| Property extends keyof This, | ||
| >(watchedFieldsOrCb: Array<keyof This[Property]> | Callback<This>, cb?: Callback<This>) { | ||
| return function (_: undefined, ctx: ClassFieldDecoratorContext<This, This[Property]>) { | ||
| const { addInitializer, name } = ctx; | ||
|
|
||
| addInitializer(function (this: This) { | ||
| const thisRoot = this; | ||
|
|
||
| const isPrimitive = isPrimitiveValue(this[name as Property]); | ||
|
|
||
| const callback = typeof watchedFieldsOrCb === 'function' ? watchedFieldsOrCb : cb!; | ||
| const fields = Array.isArray(watchedFieldsOrCb) ? watchedFieldsOrCb : null; | ||
|
|
||
| if (isPrimitive) { | ||
| let value = this[name as Property]; | ||
|
|
||
| Object.defineProperty(this, name, { | ||
| get() { | ||
| return value; | ||
| }, | ||
| set(newValue) { | ||
| const oldValue = value; | ||
|
|
||
| value = newValue; | ||
|
|
||
| if (oldValue !== newValue) { | ||
| callback.call(thisRoot, name, newValue); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd like to talk about thisRoot
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. discussed |
||
| } | ||
| }, | ||
| enumerable: true, | ||
| configurable: true, | ||
| }); | ||
| } else { | ||
| // @ts-ignore | ||
| this[name] = new Proxy(this[name], { | ||
| set(target, p, newValue) { | ||
| target[p] = newValue; | ||
|
|
||
| if (fields?.length === 0 || fields?.includes(p as keyof This[Property])) { | ||
| callback.call(thisRoot, p, newValue); | ||
| } | ||
|
|
||
| return true; | ||
| }, | ||
| }); | ||
| } | ||
| }); | ||
| }; | ||
| } | ||
|
|
||
| export default reactive; | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,56 @@ | ||||||
| type WatchedProps<Props> = { [key in keyof Props]?: any }; | ||||||
|
||||||
| type WatchedProps<Props> = { [key in keyof Props]?: any }; | |
| type WatchedProps<Props> = { [key in keyof Props]?: unknown }; |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| type Constructor<Props> = new (...args: any) => { | |
| type Constructor<Props> = new (props: Props, ...args: unknown) => { |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| render(): any; | |
| render(): React.ReactNode; |
I think we don't need this decorator on any another classes)
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do you think about using observePropsChanges or propsObserver ?
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It’s a matter of taste, but it seems like forEach would be more appropriate here.
and not this.props?, but just props from the constructor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reasonable, looks like I've been experimenting with reduce logic and it's just left untouchable for the next phase of refactoring :D
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,231 @@ | ||
| import { runDependencyCheckTests } from '@semcore/testing-utils/shared-tests'; | ||
| import { describe, it, expect } from '@semcore/testing-utils/vitest'; | ||
|
|
||
| import TimePickerEntity from '../src/entity/TimePickerEntity'; | ||
|
|
||
| describe('time-picker Dependency imports', () => { | ||
| runDependencyCheckTests('time-picker'); | ||
| }); | ||
|
|
||
| describe('TimePickerEntity', () => { | ||
| describe('constructor', () => { | ||
| it('should initialize with default empty time when no value provided', () => { | ||
| const entity = new TimePickerEntity(':', false); | ||
|
|
||
| expect(entity.hours).toBe(''); | ||
| expect(entity.minutes).toBe(''); | ||
| }); | ||
|
|
||
| it('should parse hours and minutes from value string', () => { | ||
| const entity = new TimePickerEntity('14:30', false); | ||
|
|
||
| expect(entity.hours).toBe('14'); | ||
| expect(entity.minutes).toBe('30'); | ||
| }); | ||
|
|
||
| it('should handle single digit hours and minutes', () => { | ||
| const entity = new TimePickerEntity('9:5', false); | ||
|
|
||
| expect(entity.hours).toBe('09'); | ||
| expect(entity.minutes).toBe('05'); | ||
| }); | ||
|
|
||
| it('should initialize with AM meridiem by default', () => { | ||
| const entity = new TimePickerEntity('10:00', true); | ||
|
|
||
| expect(entity.meridiem).toBe('AM'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('12-hour format', () => { | ||
| it('should format midnight (00:00) as 12:00 AM', () => { | ||
| const entity = new TimePickerEntity('0:00', true); | ||
|
|
||
| expect(entity.hours).toBe('12'); | ||
| expect(entity.minutes).toBe('00'); | ||
| }); | ||
|
|
||
| it('should format hours 1-11 AM correctly', () => { | ||
| const entity = new TimePickerEntity('9:30', true); | ||
|
|
||
| expect(entity.hours).toBe('09'); | ||
| }); | ||
|
|
||
| it('should format noon (12:00) as 12:00', () => { | ||
| const entity = new TimePickerEntity('12:00', true); | ||
|
|
||
| expect(entity.hours).toBe('12'); | ||
| }); | ||
|
|
||
| it('should format hours 13-23 as 1-11 PM', () => { | ||
| const entity = new TimePickerEntity('14:45', true); | ||
|
|
||
| expect(entity.hours).toBe('02'); | ||
| }); | ||
|
|
||
| it('should format 23:59 as 11:59', () => { | ||
|
||
| const entity = new TimePickerEntity('23:59', true); | ||
|
|
||
| expect(entity.hours).toBe('11'); | ||
| expect(entity.minutes).toBe('59'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('24-hour format', () => { | ||
| it('should format hours with leading zero in 24-hour mode', () => { | ||
| const entity = new TimePickerEntity('9:30', false); | ||
|
|
||
| expect(entity.hours).toBe('09'); | ||
| }); | ||
|
|
||
| it('should handle midnight in 24-hour format', () => { | ||
| const entity = new TimePickerEntity('0:00', false); | ||
|
|
||
| expect(entity.hours).toBe('00'); | ||
| }); | ||
|
|
||
| it('should convert 12 AM to 00:00 in 24-hour format', () => { | ||
| const entity = new TimePickerEntity('12:00', false); | ||
| entity.meridiem = 'AM'; | ||
|
|
||
| expect(entity.hours).toBe('00'); | ||
ilyabrower marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| }); | ||
|
|
||
| it('should convert 12 PM to 12:00 in 24-hour format', () => { | ||
| const entity = new TimePickerEntity('12:00', false); | ||
| entity.meridiem = 'PM'; | ||
|
|
||
| expect(entity.hours).toBe('12'); | ||
| }); | ||
|
|
||
| it('should convert PM hours correctly', () => { | ||
| const entity = new TimePickerEntity('3:00', false); | ||
| entity.meridiem = 'PM'; | ||
|
|
||
| expect(entity.hours).toBe('15'); | ||
| }); | ||
|
|
||
| it('should keep AM hours unchanged (except 12)', () => { | ||
| const entity = new TimePickerEntity('9:00', false); | ||
| entity.meridiem = 'AM'; | ||
|
|
||
| expect(entity.hours).toBe('09'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('getters and setters', () => { | ||
|
||
| it('should update hours via setter', () => { | ||
| const entity = new TimePickerEntity('10:00', false); | ||
| entity.hours = '15'; | ||
|
|
||
| expect(entity.hours).toBe('15'); | ||
| }); | ||
|
|
||
| it('should update minutes via setter', () => { | ||
| const entity = new TimePickerEntity('10:00', false); | ||
| entity.minutes = '45'; | ||
|
|
||
| expect(entity.minutes).toBe('45'); | ||
| }); | ||
|
|
||
| it('should update meridiem via setter', () => { | ||
| const entity = new TimePickerEntity('10:00', true); | ||
| entity.meridiem = 'PM'; | ||
|
|
||
| expect(entity.meridiem).toBe('PM'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('toggleMeridiem', () => { | ||
| it('should toggle from AM to PM', () => { | ||
| const entity = new TimePickerEntity('10:00', true); | ||
|
|
||
| entity.toggleMeridiem(); | ||
|
|
||
| expect(entity.meridiem).toBe('PM'); | ||
| }); | ||
|
|
||
| it('should toggle from PM to AM', () => { | ||
| const entity = new TimePickerEntity('10:00', true); | ||
| entity.meridiem = 'PM'; | ||
|
|
||
| entity.toggleMeridiem(); | ||
|
|
||
| expect(entity.meridiem).toBe('AM'); | ||
| }); | ||
|
|
||
| it('should toggle multiple times correctly', () => { | ||
| const entity = new TimePickerEntity('10:00', true); | ||
|
|
||
| entity.toggleMeridiem(); | ||
| expect(entity.meridiem).toBe('PM'); | ||
|
|
||
| entity.toggleMeridiem(); | ||
| expect(entity.meridiem).toBe('AM'); | ||
|
|
||
| entity.toggleMeridiem(); | ||
| expect(entity.meridiem).toBe('PM'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('toString', () => { | ||
| it('should return 24-hour format string when is12Hour is false', () => { | ||
| const entity = new TimePickerEntity('14:30', false); | ||
|
|
||
| expect(entity.toString()).toBe('14:30'); | ||
| }); | ||
|
|
||
| it('should convert to 24-hour format string when is12Hour is true', () => { | ||
| const entity = new TimePickerEntity('2:30', true); | ||
| entity.meridiem = 'PM'; | ||
|
|
||
| expect(entity.toString()).toBe('14:30'); | ||
| }); | ||
|
|
||
| it('should handle midnight (12 AM) conversion', () => { | ||
| const entity = new TimePickerEntity('12:00', true); | ||
| entity.meridiem = 'AM'; | ||
|
|
||
| expect(entity.toString()).toBe('00:00'); | ||
| }); | ||
|
|
||
| it('should handle noon (12 PM) conversion', () => { | ||
| const entity = new TimePickerEntity('12:00', true); | ||
| entity.meridiem = 'PM'; | ||
|
|
||
| expect(entity.toString()).toBe('12:00'); | ||
| }); | ||
|
|
||
| it('should add leading zeros to output', () => { | ||
| const entity = new TimePickerEntity('9:5', false); | ||
|
|
||
| expect(entity.toString()).toBe('09:05'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('edge cases', () => { | ||
| it('should handle invalid hours gracefully', () => { | ||
| const entity = new TimePickerEntity('invalid:30', false); | ||
|
|
||
| expect(entity.hours).toBe('invalid'); | ||
| }); | ||
|
|
||
| it('should handle empty minutes', () => { | ||
| const entity = new TimePickerEntity('10:', false); | ||
|
|
||
| expect(entity.minutes).toBe(''); | ||
| }); | ||
|
|
||
| it('should handle empty hours', () => { | ||
| const entity = new TimePickerEntity(':30', false); | ||
|
|
||
| expect(entity.hours).toBe(''); | ||
| }); | ||
|
|
||
| it('should preserve non-numeric hour values in 12-hour format', () => { | ||
| const entity = new TimePickerEntity('invalid:30', true); | ||
|
|
||
| expect(entity.hours).toBe('invalid'); | ||
| }); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,7 +9,7 @@ | |
| "author": "UI-kit team <[email protected]>", | ||
| "license": "MIT", | ||
| "scripts": { | ||
| "build": "pnpm semcore-builder --source=js && pnpm vite build" | ||
| "build": "pnpm semcore-builder && pnpm vite build" | ||
| }, | ||
| "exports": { | ||
| "require": "./lib/cjs/index.js", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can not to use
symbolhereThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
technically, we could have a
symbolas an object key :)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
technically yes, but maybe we don't need it until someone uses it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does it really worth it to delete it right now and update when someone is needed? I am fine with that :)