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.

Sign up for GitHub

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

Jump to bottom

feat: refactor batch session date picker component with react component #2215

Merged
merged 23 commits into from
Feb 19, 2024
Merged

feat: refactor batch session date picker component with react component #2215

merged 23 commits into from
Feb 19, 2024

Conversation

agatha197
Copy link
Contributor

@agatha197 agatha197 commented Feb 14, 2024 •
edited

resolves #1550

What's changed

  • replace vaadin-date-picker with react component to prevent date selection from being impossible on the calendar.
  • add a new interval text from feat/bai-interval-text which made by @yomybaby

How to test

  • open session launcher dialog.
  • change session type to 'Batch'.
  • you can see Session Starts option.

Checklist for reviewer

  • Can you create a new session without the start time setting in batch type?
  • Can you create a new session with a start time setting in batch type?
  • If you change the start time or use the default value (2 min later), is the start time set well? (you can check in the session list)
  • If you checked the Enable, is the start time set to 2 min later?
  • If you checked the Enable and didn't specify the date, the start time is not applied.
  • If you checked the Enable and set the start time to the past, you cannot move the next progress.
  • If the scheduled time goes in the past, it automatically adds 2 min.

Screenshots

  • Disabled if not click the checkbox
image
  • Normal setting
image
  • Change the status to a warning if the start time is earlier than the current time.
image
  • Enable checked, but didn't specify the date
image
  • Setting UI
image

Checklist: (if applicable)

  • Mention to the original issue
  • Documentation
  • Minium required manager version: 23.09
  • Specific setting for review (eg., KB link, endpoint or how to setup)
  • Minimum requirements to check during review
  • Test case(s) to demonstrate the difference of before/after

yomybaby and others added 15 commits February 1, 2024 10:31
@yomybaby
introduce BAIINtervalText and apply it to Neo SessionList
026e564
@yomybaby
handle neo session launcher/list by react root
6e08a15
@yomybaby
Merge remote-tracking branch 'origin/main' into feat/bai-interval-text
29ca5cc
@yomybaby
add draft version of resource group available panel
ea7b2e2
@yomybaby
Merge branch 'main' into feat/bai-interval-text
a17324e
@yomybaby
availableFilter prop of ResourceAllocationFormItems
7506c12
@yomybaby
runShmemAutomationRule when preset is changed
edf422c
@yomybaby
feat: enable allocatable resource preset only automatically
efc4b4f
@yomybaby
prevent session name autocompletion
70deb58
@agatha197
feat: batch session scheduled time setting using react component
ae8bdfe
@agatha197
feat: set schedule time when enabling checkbox and blurring
3a6b6e7
@agatha197
Merge remote-tracking branch 'origin/feat/bai-interval-text' into blo…
5341ed1 
…cker/batch-session-time-select-does-not-work
@agatha197
feat: add start timer using BAIIntervalText
0b9fc8d
@agatha197
Revert "Merge remote-tracking branch 'origin/feat/bai-interval-text' …
43a80eb 
…into blocker/batch-session-time-select-does-not-work"

This reverts commit 5341ed1, reversing
changes made to 3a6b6e7.
@agatha197
feat: patch BAIIntervalText related files from `feat/bai-interval-t…
b05f001 
…ext`
@agatha197 agatha197 added type:fix Fix features that are not working effort:normal Need to understand a few modules / some extent of contextual or historical information. area:ux UI / UX issue. urgency:blocker IT SHOULD BE RESOLVED BEFORE DISTRIBUTING urgency:4 As soon as feasible, implementation is essential. platform:general impact:visible This change is visible to users. field:UI / UX labels Feb 14, 2024
@agatha197 agatha197 added this to the 24.03 milestone Feb 14, 2024
@agatha197 agatha197 self-assigned this Feb 14, 2024
@github-actions github-actions bot added platform:web Web-specific issue urgency:3 Must be finished within a certain time frame. labels Feb 14, 2024
@github-actions github-actions bot added urgency:3 Must be finished within a certain time frame. field:localization size:L 100~500 LoC labels Feb 14, 2024
@aws-amplify-ap-northeast-2 AWS Amplify (ap-northeast-2)
Copy link

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-2215.d3g9cs6u59b8lw.amplifyapp.com

@github-actions GitHub Actions
Copy link

github-actions bot commented Feb 14, 2024 •
edited

Coverage report for ./react

St.❔
 Category Percentage Covered / Total
🔴 Statements
3.02% (-0.04% 🔻)
 102/3377
🔴 Branches
3.19% (-0.03% 🔻)
 69/2165
🔴 Functions
1.5% (-0.03% 🔻)
 17/1133
🔴 Lines
3.08% (-0.04% 🔻)
 102/3314
Show new covered files 🐣
St.❔
 File Statements Branches Functions Lines
🔴
... / BAIIntervalText.tsx
 0% 100% 0% 0%
🔴
... / BatchSessionScheduledTimeSetting.tsx
 0% 0% 0% 0%
🔴
... / useIntervalValue.tsx
 0% 0% 0% 0%
🔴
... / useEventNotStable.tsx
 0% 0% 0% 0%

Test suite run success

20 tests passing in 4 suites.

Report generated by 🧪jest coverage report action from f51355f

@gee05053
Copy link
Contributor

@agatha197

I found issue that user could set start time earlier than the current time.
Also Start After time value was calculated 1 day when user set start time to current time or earlier.
스크린샷 2024-02-15 오전 9 49 27

@github-actions github-actions bot added size:XL 500~ LoC and removed size:L 100~500 LoC labels Feb 15, 2024
@agatha197
Copy link
Contributor Author

kimsujin (@agatha197)

I found issue that user could set start time earlier than the current time. Also Start After time value was calculated 1 day when user set start time to current time or earlier. 스크린샷 2024-02-15 오전 9 49 27

resolved.

gee05053
gee05053 suggested changes Feb 16, 2024
View reviewed changes
Copy link
Contributor

@gee05053 gee05053 left a comment

Choose a reason for hiding this comment

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

Please check conflicts

gee05053
gee05053 approved these changes Feb 19, 2024
View reviewed changes
Copy link
Contributor

@gee05053 gee05053 left a comment

Choose a reason for hiding this comment

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

LGTM!

@github-actions github-actions bot added size:L 100~500 LoC and removed size:XL 500~ LoC labels Feb 19, 2024
@agatha197 agatha197 merged commit d09270a into main Feb 19, 2024
7 of 8 checks passed
@agatha197 agatha197 deleted the blocker/batch-session-time-select-does-not-work branch February 19, 2024 09:28
@yomybaby yomybaby restored the blocker/batch-session-time-select-does-not-work branch February 20, 2024 01:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gee05053 gee05053 gee05053 approved these changes

@yomybaby yomybaby Awaiting requested review from yomybaby

Assignees

@agatha197 agatha197

Labels
area:ux UI / UX issue. effort:normal Need to understand a few modules / some extent of contextual or historical information. field:localization field:UI / UX impact:visible This change is visible to users. platform:general platform:web Web-specific issue size:L 100~500 LoC type:fix Fix features that are not working urgency:blocker IT SHOULD BE RESOLVED BEFORE DISTRIBUTING urgency:3 Must be finished within a certain time frame. urgency:4 As soon as feasible, implementation is essential.
Projects
None yet
Milestone
24.03
Development

Successfully merging this pull request may close these issues.

Mouse click z-order issue in the starting date selection popup of the session creation dialog (batch-type)
3 participants
@agatha197 @gee05053 @yomybaby