dhutaryan / ngx-mat-timepicker

Material timepicker based on material design
https://dhutaryan.github.io/ngx-mat-timepicker/
MIT License
73 stars 8 forks source link

NativeDateTimeAdapter parseTime ignores AM and PM #154

Closed k3nsei closed 5 months ago

k3nsei commented 5 months ago

When initial time is for example 11:20 PM, and using keyboard to change input element value to 11:20 AM. Inputs AM/PM is set back to previous state.

It's all because of this replace https://github.com/dhutaryan/ngx-mat-timepicker/blob/78c58b5c26717c23a8a2108d02ae424b04bac9a9/projects/mat-timepicker/src/lib/adapter/native-date-time-adapter.ts#L43

Simple converter from 12h to 24h format should fix this problem.

const TIME_FORMAT_12H = /^(0[1-9]|1[0-2])(?:\.|:)([0-5][0-9])\s*(AM|PM)$/i;
const TIME_FORMAT_24H = /^([0-1][0-9]|2[0-4]):([0-5][0-9])$/i;

const parseTime = (value: string): { hour: string, minute: string } => {
    const match12H = TIME_FORMAT_12H.exec(value.trim());

    if (match12H !== null) {
        const postMeridiem = match12H[3].toUpperCase() === 'PM';
        const minute = match12H[2];
        const hour =  postMeridiem
            ? match12H[1] !== '12'
                ? `${ +match12H[1] + 12 }`
                : '12'
            : match12H[1] !== '12'
                ? match12H[1]
                : '00';

        return { hour, minute };
    }

    const match24H = TIME_FORMAT_24H.exec(value.trim());

    return {
        hour: match24H && match24H[1] !== '24' ? match24H[1] : '00',
        minute: match24H ? match24H[2] : '00'
    };
}
dhutaryan commented 5 months ago

I'd say this is more about visual than functional part. But maybe it make sense to handle am/pm as well.

k3nsei commented 5 months ago

As I"m from Europe I don't even understand why people are using this 12h in 2024. But USA is having even more stupid things like imperial system instead of metric one. Anyway I just reporting what I found migrating our project time picker to use yours library. As I'm using own Luxon based TimeAdapter it isn't affecting mine project. But reporting to make experience better for others.

dhutaryan commented 5 months ago

By the way, thank you for the issue. I really appreciate it and it helps to make the lib better.

dhutaryan commented 5 months ago

Should work in: 15.3.0 16.3.0 17.5.0

k3nsei commented 5 months ago

@dhutaryan Great, I like simplistic approach that you did there. Just one question, I didn't seen checking for out of bound values when it switches to else case. So potentially I could give bigger number than 12 and PM. Then it would jump to else cause without fallback and any error. Which could give potentially unexpected results. I didn't tested it though as its middle of night when I'm reading commit changes. I just assuming that from what I seen in commit.

Edited:

Yeap tested it and I could pass 31:42 PM or AM and it would be converted instead of giving parsing error. It's kinda ok behavior but could be source of unexpected result and surprise. I personally prefer to see validation error, but it isn't big deal as it is super minor/rare edge case.