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

Mod submitter date update #2264

Merged
merged 5 commits into from
Jun 13, 2024
Merged

Conversation

dakota002
Copy link
Contributor

Description

Updates the correction request edit form to allow for changes to Submission Date (createdOn) and Submitter

Submission Date is only allowed to be edited if it does not already exist

Related Issue(s)

Resolves #2237

Testing

  • Sign in
  • Go to a circular
  • Request a correction
  • Fill out the form

@dakota002 dakota002 requested review from lpsinger and jracusin May 6, 2024 20:38
@dakota002 dakota002 marked this pull request as ready for review May 7, 2024 14:02
package-lock.json Outdated Show resolved Hide resolved
@dakota002 dakota002 force-pushed the ModSubmitterDateUpdate branch from 6ba7269 to 831dc35 Compare May 7, 2024 19:51
@dakota002 dakota002 requested a review from lpsinger May 9, 2024 15:41
@dakota002 dakota002 force-pushed the ModSubmitterDateUpdate branch from bb37175 to ad71c16 Compare May 13, 2024 18:54
@dakota002 dakota002 requested a review from lpsinger May 13, 2024 18:55
@dakota002 dakota002 requested a review from lpsinger May 21, 2024 15:17
@dakota002 dakota002 force-pushed the ModSubmitterDateUpdate branch from 85ee49c to 534e59d Compare June 3, 2024 18:58
@dakota002
Copy link
Contributor Author

@lpsinger Okay with the latest rebase it is working

Copy link

codecov bot commented Jun 3, 2024

Codecov Report

Attention: Patch coverage is 5.55556% with 34 lines in your changes missing coverage. Please review.

Project coverage is 5.30%. Comparing base (f23abf7) to head (534e59d).
Report is 3 commits behind head on main.

Current head 534e59d differs from pull request most recent head 6cebad0

Please upload reports for the commit 6cebad0 to get more accurate results.

Files Patch % Lines
...es/circulars.edit.$circularId/CircularEditForm.tsx 0.00% 19 Missing ⚠️
app/routes/circulars._archive._index/route.tsx 0.00% 7 Missing ⚠️
...es/circulars.moderation.$circularId.$requestor.tsx 0.00% 3 Missing ⚠️
app/routes/circulars/circulars.lib.ts 50.00% 2 Missing ⚠️
app/components/TimeAgo.tsx 0.00% 1 Missing ⚠️
app/routes/circulars.correction.$circularId.tsx 0.00% 1 Missing ⚠️
app/routes/circulars/circulars.server.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##            main   #2264      +/-   ##
========================================
+ Coverage   5.28%   5.30%   +0.02%     
========================================
  Files        157     157              
  Lines       3863    3883      +20     
  Branches     405     413       +8     
========================================
+ Hits         204     206       +2     
- Misses      3658    3676      +18     
  Partials       1       1              

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

Copy link
Member

@lpsinger lpsinger left a comment

Choose a reason for hiding this comment

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

  • When the date, time, or submitter fields receive focus, the focus border should be on the input group, not the text field itself, so that it surrounds both the text field and the label. Compare with focus behavior for the subject field.
  • Implement color-coded validation borders like we have for the other fields.

Copy link
Member

@lpsinger lpsinger left a comment

Choose a reason for hiding this comment

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

We generally show times in 24 hour format on this site. Would you please set the time picker to 24 hour mode?

@dakota002
Copy link
Contributor Author

We generally show times in 24 hour format on this site. Would you please set the time picker to 24 hour mode?

I don't see that as a supported feature of this component, but I think it would be a good update later, along with the name fix for the TimePicker we were talking about above

@dakota002 dakota002 requested a review from lpsinger June 10, 2024 14:43
@dakota002
Copy link
Contributor Author

dakota002 commented Jun 10, 2024

Still working on resolving the input focus

@lpsinger This is proving more challenging than I imagined. The date and time picker inputs use the input group elements and don't really play well with the input prefix component by default. I think this can be fixed in the future, but shouldn't hold up the rest of this PR

@lpsinger
Copy link
Member

We generally show times in 24 hour format on this site. Would you please set the time picker to 24 hour mode?

I don't see that as a supported feature of this component, but I think it would be a good update later, along with the name fix for the TimePicker we were talking about above

Would you please submit an issue upstream and add a FIXME comment? 24-hour time formats are much more useful in astronomy.

@lpsinger
Copy link
Member

Still working on resolving the input focus

@lpsinger This is proving more challenging than I imagined. The date and time picker inputs use the input group elements and don't really play well with the input prefix component by default. I think this can be fixed in the future, but shouldn't hold up the rest of this PR

Let me take one quick swing at this and see if I can figure it out. Would you please rebase your branch from main?

Copy link
Member

@lpsinger lpsinger left a comment

Choose a reason for hiding this comment

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

Please add a validation border around the From field.

@dakota002 dakota002 force-pushed the ModSubmitterDateUpdate branch from 0976944 to fab6ff4 Compare June 12, 2024 12:57
Updates the moderation form to allow edits of createdOn and submitter

Adds some validations

Include HH:MM in time selection

UTC, Grid, and fix button height

refactor to reuse TimeAgo, tidy up date time checks

Date edit refactoring

rename fields for consistency, add hidden field to fix time not getting set correctly

Remove date replacement, still need to test against latest update

Remove FIXME, reorder of submitter check for consistency, reorder createdOn and submitter to minimize changes

Update app/routes/circulars._archive._index/route.tsx

Co-authored-by: Leo Singer <[email protected]>

Style updates for consistency, spacing, other PR feedback implementations

refine the cirteria for error or success class on date string
@dakota002 dakota002 force-pushed the ModSubmitterDateUpdate branch from fab6ff4 to 56a7695 Compare June 12, 2024 13:02
@dakota002
Copy link
Contributor Author

Still working on resolving the input focus
@lpsinger This is proving more challenging than I imagined. The date and time picker inputs use the input group elements and don't really play well with the input prefix component by default. I think this can be fixed in the future, but shouldn't hold up the rest of this PR

Let me take one quick swing at this and see if I can figure it out. Would you please rebase your branch from main?

Done!

Copy link
Member

@lpsinger lpsinger left a comment

Choose a reason for hiding this comment

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

Would you please make my suggestions from dakota002#314 about only putting the date and time picker in a Grid and not the entire component?

@dakota002
Copy link
Contributor Author

dakota002 commented Jun 12, 2024

Would you please make my suggestions from dakota002#314 about only putting the date and time picker in a Grid and not the entire component?

Woops, I missed that one, merged in now

Updates the moderation form to allow edits of createdOn and submitter

Adds some validations

Include HH:MM in time selection

UTC, Grid, and fix button height

refactor to reuse TimeAgo, tidy up date time checks

Date edit refactoring

rename fields for consistency, add hidden field to fix time not getting set correctly

Remove date replacement, still need to test against latest update

Remove FIXME, reorder of submitter check for consistency, reorder createdOn and submitter to minimize changes

Clean up grid stuff
app/routes/circulars/circulars.lib.ts Outdated Show resolved Hide resolved
@dakota002 dakota002 requested a review from lpsinger June 13, 2024 13:31
@lpsinger lpsinger merged commit 0726e62 into nasa-gcn:main Jun 13, 2024
10 checks passed
@lpsinger
Copy link
Member

I get an HTTP 400 error when I submit the correction request form if I am not a moderator. Please fix.

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.

edit submitter name/email and date in moderator interface
2 participants