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

front: fix study dates #5501

Merged
merged 1 commit into from
Nov 10, 2023
Merged

front: fix study dates #5501

merged 1 commit into from
Nov 10, 2023

Conversation

Akctarus
Copy link
Contributor

closes #5479 for the front part

@Akctarus Akctarus requested a review from a team as a code owner October 26, 2023 16:38
@codecov
Copy link

codecov bot commented Oct 26, 2023

Codecov Report

Merging #5501 (c4acd64) into dev (1bfe9fd) will decrease coverage by 0.01%.
The diff coverage is 9.83%.

@@             Coverage Diff              @@
##                dev    #5501      +/-   ##
============================================
- Coverage     19.60%   19.60%   -0.01%     
  Complexity     2322     2322              
============================================
  Files           885      886       +1     
  Lines        105755   105787      +32     
  Branches       2571     2572       +1     
============================================
+ Hits          20731    20735       +4     
- Misses        83516    83543      +27     
- Partials       1508     1509       +1     
Flag Coverage Δ
front 8.34% <9.83%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
front/src/common/BootstrapSNCF/InputSNCF.tsx 25.27% <100.00%> (ø)
front/src/utils/date.ts 59.37% <36.36%> (-12.06%) ⬇️
...c/modules/study/components/AddOrEditStudyModal.tsx 0.00% <0.00%> (ø)
front/src/modules/study/utils.ts 0.00% <0.00%> (ø)

@Akctarus Akctarus force-pushed the tmn/front-fix-study-date branch 4 times, most recently from ced1f65 to 16d3e7a Compare October 27, 2023 09:06
Copy link
Contributor

@SarahBellaha SarahBellaha left a comment

Choose a reason for hiding this comment

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

LGTM :)
Just left a tiny suggestion

@Akctarus Akctarus force-pushed the tmn/front-fix-study-date branch from 16d3e7a to 042d532 Compare October 30, 2023 13:27
@Akctarus Akctarus force-pushed the tmn/front-fix-study-date branch 2 times, most recently from 6db1531 to de18bdf Compare November 2, 2023 12:49
@Akctarus Akctarus force-pushed the tmn/front-fix-study-date branch 2 times, most recently from 9347b1e to 579aef1 Compare November 3, 2023 11:41
Copy link
Contributor

@SharglutDev SharglutDev left a comment

Choose a reason for hiding this comment

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

Tested in local, left a suggestion to simplify the code.

According the front guidelines, you could also move this function, the formatDateForInput and createSelectOptions in a utils file in the study folder.

@Akctarus Akctarus force-pushed the tmn/front-fix-study-date branch 9 times, most recently from 0f262e7 to a99d6ab Compare November 3, 2023 16:34
Copy link
Contributor

@SharglutDev SharglutDev left a comment

Choose a reason for hiding this comment

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

image

Here both dates are underlined in red but only one is invalid.
I think it should be only the date responsible for the invalidity that gets the red underline

@Akctarus Akctarus force-pushed the tmn/front-fix-study-date branch from a99d6ab to e9e331e Compare November 6, 2023 16:27
@Akctarus Akctarus force-pushed the tmn/front-fix-study-date branch from e9e331e to 23cc0c9 Compare November 6, 2023 21:50
@Akctarus Akctarus force-pushed the tmn/front-fix-study-date branch from 23cc0c9 to f805321 Compare November 7, 2023 09:45
Copy link
Contributor

@SharglutDev SharglutDev left a comment

Choose a reason for hiding this comment

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

Lgtm, great job :)

@Akctarus Akctarus force-pushed the tmn/front-fix-study-date branch 4 times, most recently from b219214 to 179ac12 Compare November 9, 2023 14:13
Copy link
Contributor

@clarani clarani left a comment

Choose a reason for hiding this comment

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

LGTM ✅

@Akctarus Akctarus force-pushed the tmn/front-fix-study-date branch from 179ac12 to c4acd64 Compare November 10, 2023 12:30
@Akctarus Akctarus enabled auto-merge November 10, 2023 12:31
@Akctarus Akctarus added this pull request to the merge queue Nov 10, 2023
Merged via the queue into dev with commit a0c9aab Nov 10, 2023
@Akctarus Akctarus deleted the tmn/front-fix-study-date branch November 10, 2023 12:42
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.

Inconsistent dates can be entered
5 participants