Skip to content
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

NAS-132423 / 25.04 / Add ix-datepicker component #11090

Merged
merged 6 commits into from
Nov 27, 2024
Merged

NAS-132423 / 25.04 / Add ix-datepicker component #11090

merged 6 commits into from
Nov 27, 2024

Conversation

denysbutenko
Copy link
Member

@denysbutenko denysbutenko commented Nov 22, 2024

Changes:

Add ix-datepicker component.

Testing:

For testing, check My API Keys page and try create or update an API key.

@denysbutenko denysbutenko requested a review from a team as a code owner November 22, 2024 12:52
@denysbutenko denysbutenko requested review from AlexKarpov98 and undsoft and removed request for a team November 22, 2024 12:52
@bugclerk
Copy link
Contributor

@bugclerk bugclerk changed the title NAS-132423: Add ix-datepicker component NAS-132423 / 25.04 / Add ix-datepicker component Nov 22, 2024
Copy link

codecov bot commented Nov 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.35%. Comparing base (7ad0021) to head (2ac7a5b).
Report is 2 commits behind head on master.

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #11090      +/-   ##
==========================================
+ Coverage   82.34%   82.35%   +0.01%     
==========================================
  Files        1643     1647       +4     
  Lines       57484    57595     +111     
  Branches     5930     5937       +7     
==========================================
+ Hits        47333    47433     +100     
- Misses      10151    10162      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@AlexKarpov98 AlexKarpov98 left a comment

Choose a reason for hiding this comment

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

It looks pretty good to me.

One thing I noticed is that I can set 2020 year for example, form is counted as valid, but date picker dialog shows 2024 year instead (because 2020 is blocked - but actually selected via input)

Screen.Recording.2024-11-25.at.14.04.51.mov

As for me icon is too big:
Screenshot 2024-11-25 at 14 08 21

Let's see what (maestro) @undsoft thoughts.

[tooltip]="tooltips.expires | translate"
[required]="true"
></ix-input>
<ix-datepicker
Copy link
Contributor

@AlexKarpov98 AlexKarpov98 Nov 25, 2024

Choose a reason for hiding this comment

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

Let's use (it's not for the form but for the table I mean)

    relativeDateColumn({
      title: this.translate.instant('Expires Date'),
      propertyName: 'expires_at',
      getValue: (row) => row.expires_at?.$date || this.translate.instant('Never'),
    }),

For table, to see date in another format, like
Screenshot 2024-11-25 at 14 05 18

@undsoft
Copy link
Collaborator

undsoft commented Nov 26, 2024

I've updated the PR to properly handle differences in timezones and custom date formats user may have in settings.

Copy link
Contributor

@AlexKarpov98 AlexKarpov98 left a comment

Choose a reason for hiding this comment

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

👍

@undsoft undsoft merged commit 485593d into master Nov 27, 2024
11 checks passed
@undsoft undsoft deleted the NAS-132423 branch November 27, 2024 09:11
@bugclerk
Copy link
Contributor

This PR has been merged and conversations have been locked.
If you would like to discuss more about this issue please use our forums or raise a Jira ticket.

@truenas truenas locked as resolved and limited conversation to collaborators Nov 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants