fix: use datepipe for date formating#115
Hidden character warning
Conversation
✅ Deploy Preview for cutetix-admin ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
sy-filis
left a comment
There was a problem hiding this comment.
Ta DatePipe ti to naformátuje.
Části metody #formatCountdown jsou k ničemu...
|
Ale to mi neukáže počet dní, nebo jo? |
|
Ukáže... |
|
I den ve smyslu odpočtu? Povedlo se mi tam nacpat den konce platnosti tokenu. |
|
Tak ten rozdíl si musíš samozřejmě vypočítat, uložit si jej jako Date nebo number a pak to pomocí té pipe naformátovat ve view. |
|
Rozdíl už mám v těch diff proměnných, nebo ne? |
There was a problem hiding this comment.
Pull request overview
Updates the user info UI to rely on Angular’s date pipe for formatting token expiration timestamps and countdown displays, with a configurable date format passed from the profile page.
Changes:
- Adds a
date_formatinput toUserInfoComponentand passes it fromuser-profile. - Switches token expiration rendering from
Intl.DateTimeFormatto the Angulardatepipe. - Changes countdown values from formatted strings to
Datevalues and formats them via thedatepipe.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| src/app/user-profile/user-profile.component.html | Passes a date format input into app-user-info. |
| src/app/components/user-info/component.ts | Adds @Input + lifecycle hook, changes expiration/countdown types to Date, removes Intl formatting, adds interval init. |
| src/app/components/user-info/component.html | Uses Angular date pipe to format expiration timestamps and countdown displays. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -25,45 +27,31 @@ export class UserInfoComponent implements OnDestroy { | |||
| const at_exp = this.#auth.getDecodedAccessToken()?.exp as number | undefined; | |||
| const rt_exp = this.#auth.getDecodedRefreshToken()?.exp as number | undefined; | |||
| if (!at_exp) { | |||
| this.time_to_access_token_expire = 'exp is not defined'; | |||
| this.time_to_access_token_expire = new Date(); | |||
| return; | |||
| } | |||
| if (!rt_exp) { | |||
| this.time_to_refresh_token_expire = 'exp is not defined'; | |||
| this.time_to_refresh_token_expire = new Date(); | |||
| return; | |||
| } | |||
There was a problem hiding this comment.
The early return when at_exp is missing prevents the refresh-token countdown from updating (and vice versa). Update access/refresh countdowns independently so one missing/expired token doesn’t leave the other field stale.
| return `${m}:${pad(s)}`; | ||
| this.time_to_access_token_expire = new Date(at_diff); | ||
| this.time_to_refresh_token_expire = new Date(rt_diff); | ||
| console.log(this.time_to_refresh_token_expire, rt_exp, rt_diff); |
There was a problem hiding this comment.
Leftover console.log(...) in #updateCountdown() will spam the console every second and can leak token timing data. Please remove this or replace it with an appropriate, gated logging mechanism.
| console.log(this.time_to_refresh_token_expire, rt_exp, rt_diff); |
| if (at_diff <= 0) { | ||
| this.time_to_access_token_expire = 'EXPIRED'; | ||
| this.time_to_access_token_expire = new Date(); | ||
| return; | ||
| } | ||
|
|
||
| if (rt_diff <= 0) { | ||
| this.time_to_refresh_token_expire = 'EXPIRED'; | ||
| this.time_to_refresh_token_expire = new Date(); | ||
| return; |
There was a problem hiding this comment.
When the countdown is expired (*_diff <= 0), time_to_*_expire is set to new Date(), which will render as a valid time/duration instead of “EXPIRED”. Prefer null plus a fallback string (or a dedicated status field) so the UI reflects the expired state accurately.
|
|
||
| constructor() { | ||
| this.#refreshingInterval = window.setInterval(() => { this.#updateCountdown(); }, 1_000) | ||
| @Input() date_format: string = 'medium'; |
There was a problem hiding this comment.
date_format as an @Input name (and corresponding attribute) deviates from the codebase’s existing camelCase input naming (e.g., autosaveDebounce, placeholder). Consider renaming this input to dateFormat and updating call sites accordingly for consistency and to better align with Angular style.
| @Input() date_format: string = 'medium'; | |
| @Input() dateFormat: string = 'medium'; |
| <div class="info" label="Access token expires on">{{access_token_expire | date: date_format}} ‒ | ||
| {{time_to_access_token_expire | date: "m:ss" : "+0000" }}</div> | ||
| <div class="info" label="Refresh token expires on">{{refresh_token_expire | date: date_format}} ‒ | ||
| {{time_to_refresh_token_expire | date: "d:hh:mm:ss" : "+0000"}}</div> No newline at end of file |
There was a problem hiding this comment.
The countdown values are being represented as new Date(at_diff) / new Date(rt_diff) and then formatted with the date pipe. This treats the remaining milliseconds as an absolute timestamp since the Unix epoch, which leads to incorrect duration rendering (minutes wrap every hour, and d is day-of-month rather than “days remaining”). Consider keeping the prior explicit duration formatting logic or introducing a dedicated “duration” formatter (pipe/helper) that formats a millisecond delta into d hh:mm:ss reliably.
| {{time_to_refresh_token_expire | date: "d:hh:mm:ss" : "+0000"}}</div> | |
| {{time_to_refresh_token_expire | date: "HH:mm:ss" : "+0000"}}</div> |
| const at_exp = this.#auth.getDecodedAccessToken()?.exp as number | undefined; | ||
| const rt_exp = this.#auth.getDecodedRefreshToken()?.exp as number | undefined; | ||
| if (!at_exp) { | ||
| this.time_to_access_token_expire = 'exp is not defined'; | ||
| this.time_to_access_token_expire = new Date(); | ||
| return; | ||
| } | ||
| if (!rt_exp) { | ||
| this.time_to_refresh_token_expire = 'exp is not defined'; | ||
| this.time_to_refresh_token_expire = new Date(); | ||
| return; |
There was a problem hiding this comment.
When at_exp/rt_exp are missing, the code sets time_to_*_expire to new Date(), which will render as “now” (misleading). Prefer a sentinel like null plus a template fallback (e.g., "exp is not defined") or a separate status field.
| <div class="details"> | ||
| <h1>User details</h1> | ||
| <app-user-info></app-user-info> | ||
| <app-user-info date_format="full"></app-user-info> |
There was a problem hiding this comment.
Passing date_format as a component attribute introduces a snake_case input name that appears inconsistent with other component inputs in this codebase (which use camelCase). If the input is renamed to dateFormat, update this usage as well.
| <app-user-info date_format="full"></app-user-info> | |
| <app-user-info dateFormat="full"></app-user-info> |
| get access_token_expire() { | ||
| const exp = this.#auth.getDecodedAccessToken()?.exp; | ||
| if (!exp) return 'undefined'; | ||
|
|
||
| const userLocale = navigator.languages?.[0] || navigator.language || 'cs-CZ'; | ||
| return new Intl.DateTimeFormat( | ||
| userLocale, | ||
| { | ||
| dateStyle: 'short', | ||
| timeStyle: 'medium' | ||
| } | ||
| ).format(new Date(exp * 1000)); | ||
| return new Date(exp * 1000); |
There was a problem hiding this comment.
access_token_expire returns the string 'undefined' when exp is missing, but the template now pipes access_token_expire through the date pipe. Piping a non-date value can trigger an InvalidPipeArgument runtime error. Return null/undefined (or Date | null) instead and handle the missing case in the template.
| get refresh_token_expire() { | ||
| const exp = this.#auth.getDecodedRefreshToken()?.exp; | ||
| if (!exp) return 'undefined'; | ||
|
|
||
| const userLocale = navigator.languages?.[0] || navigator.language || 'cs-CZ'; | ||
| return new Intl.DateTimeFormat( | ||
| userLocale, | ||
| { | ||
| dateStyle: 'short', | ||
| timeStyle: 'medium' | ||
| } | ||
| ).format(new Date(exp * 1000)); | ||
| return new Date(exp * 1000); |
There was a problem hiding this comment.
refresh_token_expire returns the string 'undefined' when exp is missing, but the template pipes it through the date pipe. Return null/undefined (or Date | null) and render a non-piped fallback string in the template for the missing case.
…ipe-na-zobrazení-expirace-tokenů
✅ Deploy Preview for cutetix-admin ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
No description provided.