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

feat: add DateSelector dateFormat prop #2726

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

lpsinger
Copy link
Contributor

Summary

Related Issues or PRs

How To Test

Screenshots (optional)

@lpsinger lpsinger requested a review from a team as a code owner January 22, 2024 18:16
@lpsinger lpsinger force-pushed the dateFormat branch 2 times, most recently from 5a98f48 to 86034fb Compare January 22, 2024 20:48
@lpsinger lpsinger changed the title Add DateSelector dateFormat prop feat: add DateSelector dateFormat prop Jan 22, 2024
Copy link
Contributor

@werdnanoslen werdnanoslen left a comment

Choose a reason for hiding this comment

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

This seems useful — I'd just recommend updating DatePicker.stories.tsx to demonstrate how to use it. Do you mind if I commit to this branch with the story addition?

@lpsinger
Copy link
Contributor Author

lpsinger commented Feb 8, 2024

This seems useful — I'd just recommend updating DatePicker.stories.tsx to demonstrate how to use it. Do you mind if I commit to this branch with the story addition?

Go for it!

@werdnanoslen werdnanoslen reopened this Feb 8, 2024
@lpsinger lpsinger requested a review from a team as a code owner February 8, 2024 16:55
@werdnanoslen
Copy link
Contributor

This seems useful — I'd just recommend updating DatePicker.stories.tsx to demonstrate how to use it. Do you mind if I commit to this branch with the story addition?

Go for it!

Pardon the accidental close, here's the commit

AnnaGingle
AnnaGingle previously approved these changes Feb 15, 2024
Copy link

@AnnaGingle AnnaGingle left a comment

Choose a reason for hiding this comment

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

LGTM!

@lpsinger
Copy link
Contributor Author

Any update on this?

@werdnanoslen
Copy link
Contributor

Any update on this?

Just waiting on code reviews from our engineers (for this and the other PRs), pardon the delay

lpsinger added a commit to lpsinger/gcn.nasa.gov that referenced this pull request May 27, 2024
- uswds/uswds#5825 has been fixed.
- https://github.com/lpsinger/react-uswds/tree/gcn is the same as
  trussworks/react-uswds#2726, but rebased.
- I had to update some CSS where we have heavily customized the
  appearance of some USWDS components.
@lpsinger
Copy link
Contributor Author

I fixed a form validation bug. Please review again.

lpsinger added a commit to lpsinger/gcn.nasa.gov that referenced this pull request May 31, 2024
- uswds/uswds#5825 has been fixed.
- https://github.com/lpsinger/react-uswds/tree/gcn is the same as
  trussworks/react-uswds#2726, but rebased.
- I had to update some CSS where we have heavily customized the
  appearance of some USWDS components.
lpsinger added a commit to nasa-gcn/gcn.nasa.gov that referenced this pull request Jun 1, 2024
- uswds/uswds#5825 has been fixed.
- https://github.com/lpsinger/react-uswds/tree/gcn is the same as
  trussworks/react-uswds#2726, but rebased.
- I had to update some CSS where we have heavily customized the
  appearance of some USWDS components.
@lpsinger
Copy link
Contributor Author

lpsinger commented Sep 3, 2024

I rebased this again. Any updates on when someone will review this?

@lpsinger
Copy link
Contributor Author

lpsinger commented Dec 4, 2024

It's been a few more months since the last time I checked on this. Would someone please review it?

@@ -34,3 +34,6 @@ export const YEAR_CHUNK = 12
export const DEFAULT_MIN_DATE = '0000-01-01'
export const DEFAULT_EXTERNAL_DATE_FORMAT = 'MM/DD/YYYY'
Copy link
Contributor

@shkeating shkeating Dec 6, 2024

Choose a reason for hiding this comment

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

nitpick: I am good with passing the Happo diffs if you can explain why the switch to all caps for the help text? my thoughts would be to not change the default text if it can be avoided

(I am not at Truss anymore but will see if I can reach out to someone who is with Codeowner status to look at this, sorry about the wait!)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know why the case changed. I see what you mean, but I don't see anything in my patch that would cause it to do that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feat] Modifiable external_date_format
5 participants