feat: new `Time`-mask (`kit`) + new `overwriteMode`-mode (`core`) #37

nsbarsukov · 2022-12-08T14:14:22Z

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows Conventional Commits
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

Closes #30

github-actions · 2022-12-08T14:18:16Z

Visit the preview URL for this PR (updated for commit 120ca80):

https://maskito--pr37-time-mask-a5dx300z.web.app

_{(expires Wed, 14 Dec 2022 10:09:01 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

_{Sign: 61e4dea776cbea516b68c67840913d2edd88bb90}

projects/core/src/lib/mask.ts

waterplea · 2022-12-13T03:09:44Z

Pressing '2' does not do anything, should move caret behind '2'.

waterplea · 2022-12-13T03:14:47Z

Pressing 1 moves all other digits to the left, making them invalid and thus drops all remaining characters. Not sure this is the best way to handle this. Maybe replace second char with 0 so numbers don't shift?

waterplea · 2022-12-13T03:15:44Z

Pressing 1 doesn't do anything.

waterplea · 2022-12-13T03:16:24Z

I have a feeling that in case of time we want to fill blank spaces with 0, otherwise it looks rather weird. What do you guys think?

waterplea · 2022-12-13T03:00:24Z

projects/kit/src/lib/time/utils/parse-time-string.ts

+/**
+ * @param timeString can be with/without fixed characters
+ */
+export function parseTimeString(timeString: string): MaskitoTimeSegments {


This function looks like it works with hours down to milliseconds with optional precision, but what if we want to go the other way around and store seconds + milliseconds and optionally add minutes and hours?

#37 (comment)

waterplea · 2022-12-13T03:00:56Z

projects/kit/src/lib/time/types/time-segments.ts

@@ -0,0 +1,6 @@
+export interface MaskitoTimeSegments<T = string> {


What's the point of generic here? Shouldn't it just be string | number?

This type is used many times in the code.
Sometimes it is convenient to have object with all strings. Sometimes with all numbers.
And don't do extra type checks.

waterplea · 2022-12-13T03:01:56Z

projects/kit/src/lib/time/types/time-mode.ts

@@ -0,0 +1 @@
+export type MaskitoTimeMode = 'HH:MM' | 'HH:MM:SS' | 'HH:MM:SS.MSS';


That's what I asked about in parsing function. I think we should also support SS.MSS and MM.SS.MSS. Maybe in the next iteration, but keep that in mind.

Okey, let's keep it in mind for the next iteration.

For the first iteration, all kit-masks should reproduce already existing Taiga UI features + fixing old bugs.
All new features – next iteration.

🚀 - Time supports SS.MSS & MM.SS.MSS modes #38

waterplea · 2022-12-13T03:03:27Z

projects/kit/src/lib/time/constants/default-time-segment-max-values.ts

+import {MaskitoTimeSegments} from '../types';
+
+export const DEFAULT_TIME_SEGMENT_MAX_VALUES: MaskitoTimeSegments<number> = {
+    hours: 23,


Just something to keep in mind, we'd want to support AM/PM mode eventually too.

#37 (comment)

🚀 - Time supports SS.MSS & MM.SS.MSS modes #38

waterplea · 2022-12-13T03:20:40Z

projects/core/src/lib/mask.ts

-            elementState.value.slice(0, from) +
-            insertedText +
-            elementState.value.slice(to);
+            elementState.value.slice(0, from) + data + elementState.value.slice(to);


What is the difference between insertedText and data in this case?

data - inserted characters BEFORE preprocessing (raw data from Event.data).

insertedText - inserted characters AFTER preprocessing

We calculate newPossibleValue to decide if we have to prevent native event? Will natively typed character make mask valid (without programatic interference)?

It is especially useful for RegExp-mask (without fixed characters masks).
The majority of all typed characters can be inserted natively (without programatic value patching) ?

waterplea · 2022-12-13T03:21:25Z

projects/demo/src/pages/kit/time/time-mask-doc.component.ts

+        'HH:MM:SS.MSS': maskitoTimeOptionsGenerator({mode: 'HH:MM:SS.MSS'}),
+        'HH:MM 12-hours': maskitoTimeOptionsGenerator({
+            mode: 'HH:MM',
+            timeSegmentMaxValues: {hours: 12},


Maybe just maxValues?

In my opinion, It is not obvious for user what maxValues-option means :(

For example, maxValues: {HH: 11, MM: 30}. Okey, 11:31 is obviously invalid value. What's about 10:31 ?

projects/kit/src/lib/time/processors/zero-placeholders-preprocessor.ts

projects/kit/src/lib/time/time-mask.ts

waterplea · 2022-12-13T03:32:03Z

projects/kit/src/lib/time/processors/zero-padding-postprocessor.ts

+    };
+}
+
+function padWithZeroes(timeSegment: string, maxSegmentValue: string): string {


Shouldn't it just be padStart somehow?

It can be possible if all time segments are the same length (but they can be 2 or 3-character length).

That is why I need recursive function here.

nsbarsukov self-assigned this Dec 8, 2022

splincode approved these changes Dec 8, 2022

View reviewed changes

nsbarsukov force-pushed the time-mask branch 3 times, most recently from efef971 to 70a0516 Compare December 9, 2022 15:36

feat: new Time-mask (kit) + new overwriteMode-mode (core)

87087e6

nsbarsukov force-pushed the time-mask branch from 29ff827 to 87087e6 Compare December 12, 2022 10:39

nsbarsukov changed the title ~~[WIP] feat(kit): new Time-mask~~ [WIP] feat: new Time-mask (kit) + new overwriteMode-mode (core) Dec 12, 2022

feat(kit): Time add createMaxValidationPreprocessor

4546d60

nsbarsukov force-pushed the time-mask branch from a3d5b1f to 4546d60 Compare December 12, 2022 13:45

refactor: small changes

1182e1a

nsbarsukov changed the title ~~[WIP] feat: new Time-mask (kit) + new overwriteMode-mode (core)~~ feat: new Time-mask (kit) + new overwriteMode-mode (core) Dec 12, 2022

nsbarsukov requested review from waterplea and vladimirpotekhin December 12, 2022 14:06

nsbarsukov marked this pull request as ready for review December 12, 2022 14:08

nsbarsukov requested a review from splincode December 12, 2022 14:09

splincode reviewed Dec 12, 2022

View reviewed changes

projects/core/src/lib/mask.ts Show resolved Hide resolved

splincode approved these changes Dec 12, 2022

View reviewed changes

waterplea reviewed Dec 13, 2022

View reviewed changes

fix(kit): Time select text range + press new digit

aa28b8d

nsbarsukov force-pushed the time-mask branch from d977517 to aa28b8d Compare December 13, 2022 09:17

nsbarsukov mentioned this pull request Dec 13, 2022

🚀 - Time supports SS.MSS & MM.SS.MSS modes #38

Closed

feat(kit): Time add tailing zero-placeholders

120ca80

nsbarsukov force-pushed the time-mask branch from 10bf574 to 120ca80 Compare December 13, 2022 10:07

nsbarsukov requested a review from splincode December 13, 2022 10:14

splincode approved these changes Dec 14, 2022

View reviewed changes

waterplea approved these changes Dec 14, 2022

View reviewed changes

nsbarsukov merged commit 43dea21 into main Dec 14, 2022

nsbarsukov deleted the time-mask branch December 14, 2022 09:41

This was referenced Feb 14, 2023

🚀 Release/v0.1.0 #118

Closed

🚀 Release/v0.1.0 #121

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: new `Time`-mask (`kit`) + new `overwriteMode`-mode (`core`) #37

feat: new `Time`-mask (`kit`) + new `overwriteMode`-mode (`core`) #37

nsbarsukov commented Dec 8, 2022

github-actions bot commented Dec 8, 2022 •

edited

Loading

waterplea commented Dec 13, 2022

waterplea commented Dec 13, 2022

waterplea commented Dec 13, 2022

waterplea commented Dec 13, 2022

waterplea Dec 13, 2022

nsbarsukov Dec 13, 2022

waterplea Dec 13, 2022

nsbarsukov Dec 13, 2022

waterplea Dec 13, 2022

nsbarsukov Dec 13, 2022

nsbarsukov Dec 13, 2022

waterplea Dec 13, 2022

nsbarsukov Dec 13, 2022

nsbarsukov Dec 13, 2022 •

edited

Loading

waterplea Dec 13, 2022

nsbarsukov Dec 13, 2022 •

edited

Loading

waterplea Dec 13, 2022

nsbarsukov Dec 13, 2022

waterplea Dec 13, 2022

nsbarsukov Dec 13, 2022

		@@ -0,0 +1,6 @@
		export interface MaskitoTimeSegments<T = string> {

		@@ -0,0 +1 @@
		export type MaskitoTimeMode = 'HH:MM' \| 'HH:MM:SS' \| 'HH:MM:SS.MSS';

feat: new Time-mask (kit) + new overwriteMode-mode (core) #37

feat: new Time-mask (kit) + new overwriteMode-mode (core) #37

Conversation

nsbarsukov commented Dec 8, 2022

PR Checklist

PR Type

github-actions bot commented Dec 8, 2022 • edited Loading

waterplea commented Dec 13, 2022

waterplea commented Dec 13, 2022

waterplea commented Dec 13, 2022

waterplea commented Dec 13, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nsbarsukov Dec 13, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nsbarsukov Dec 13, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

feat: new `Time`-mask (`kit`) + new `overwriteMode`-mode (`core`) #37

feat: new `Time`-mask (`kit`) + new `overwriteMode`-mode (`core`) #37

github-actions bot commented Dec 8, 2022 •

edited

Loading

nsbarsukov Dec 13, 2022 •

edited

Loading

nsbarsukov Dec 13, 2022 •

edited

Loading