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

[#11571] Selective Deadline Extensions - Frontend tables and modals #11603

Merged
merged 75 commits into from
Apr 11, 2022
Merged

[#11571] Selective Deadline Extensions - Frontend tables and modals #11603

merged 75 commits into from
Apr 11, 2022

Conversation

FergusMok
Copy link
Contributor

@FergusMok FergusMok commented Feb 27, 2022 •
edited by samuelfangjw
Loading

Part of #11571 for Milestone 1

This PR includes

  • Instructor's individual extension page for students and instructors
  • Selection of extension time modal
  • Confirmation modal
firefox_O7WonoxYsO.mp4

Current behavior of extension:

  1. You should be able to select individuals with and without extensions to update / create extension at the same time.
  2. You should only be able to select individuals with pre-existing extensions to delete.

The frontend has additional validation for:
1. Moving the sessionEndTime to before any extensions (in the sessions edit page), we'll warn of the invalidation of some extensions. - Will be handled in a separate PR #11704
2. Deleting Extensions / Updating pre-existing extensions to before the current date (in the extension page), users are allowed but are warned of immediate closure of the session for affected individuals. - This is handled in the simple modal in individual-extension-date-modal.component
3. Deleting Extensions / Updating pre-existing extensions to before the current sessionEndTime - Users are not allowed to do this. The customize Extend-By options, and Extend-To date-picker will not allow this.

To be handled in a separate PR:

  1. Toggleable tables for the instructors / students [#11571] Selective Deadline Extensions - Add collapsible tables to extension page #11692
  2. Status of extensions for the feedback session from the feedback session page [#11571] Selective Deadline Extensions - Add extension status to feedback session views #11656

@FergusMok FergusMok added the s.Ongoing The PR is being worked on by the author(s) label Feb 27, 2022
@moziliar moziliar force-pushed the selective-deadline-extension branch 2 times, most recently from 2a5ba43 to 798c4a5 Compare March 2, 2022 12:44
@moziliar
Copy link
Contributor

moziliar commented Mar 4, 2022

Would be good to also include a snapshot of the UI in the PR.

@moziliar
Copy link
Contributor

moziliar commented Mar 9, 2022

@FergusMok Any updates on this PR?

@FergusMok
Copy link
Contributor Author

@moziliar Yup, sorry for the delay. I updated the PR with a short video on the UI

@samuelfangjw
Copy link
Member

@FergusMok Seems like package-lock.json is modified, could you help check if this is intended?

@FergusMok
Copy link
Contributor Author

@samuelfangjw Yup thanks for pointing that out. It was unintentional

@FergusMok FergusMok marked this pull request as draft March 10, 2022 08:20
@moziliar
Copy link
Contributor

The UI seems to honour the tech design!

Some comments I have about the UI:

  • Will the dropdown bar look nicer if it is shorter?
  • For the text below the dropdown: should it be "The deadline will be extended from T1 to T2" so that the user can refer to the old one quicker?
  • Will the confirmation page with the students affected be included too?

Can also look into how E2E can be done.

@moziliar moziliar force-pushed the selective-deadline-extension branch from 798c4a5 to deaac10 Compare March 11, 2022 02:13
@FergusMok FergusMok marked this pull request as ready for review March 19, 2022 05:12
@samuelfangjw
Copy link
Member

samuelfangjw commented Mar 19, 2022 •
edited
Loading

@FergusMok, can help to rebase only the relevant changes onto selective deadline ext branch? Shouldn't include irrelevant commits from master.

@FergusMok FergusMok closed this Mar 19, 2022
@FergusMok FergusMok reopened this Mar 19, 2022
@FergusMok
Copy link
Contributor Author

@moziliar Hi Zongran

Some comments I have about the UI:

* Will the dropdown bar look nicer if it is shorter?

Are you talking about the options for "Extend By"?

* For the text below the dropdown: should it be "The deadline will be extended from T1 to T2" so that the user can refer to the old one quicker?

This makes sense, but it'll be impossible to get T1 if we are trying to extend multiple individuals with pre-existing extensions

* Will the confirmation page with the students affected be included too?

Yup, the confirmation page is the 2nd modal

@FergusMok FergusMok force-pushed the fergus/mockup branch 2 times, most recently from e44a458 to 060ddc9 Compare March 19, 2022 06:50
@FergusMok FergusMok force-pushed the fergus/mockup branch 2 times, most recently from 495b71a to 87c7892 Compare March 19, 2022 07:17
@FergusMok
Copy link
Contributor Author

@samuelfangjw Yup thanks for the catch, I just rebased

@moziliar
Copy link
Contributor

Are you talking about the options for "Extend By"?

Yep. It looks unnecessarily long?

This makes sense, but it'll be impossible to get T1 if we are trying to extend multiple individuals with pre-existing extensions

I see. In that case, just leave it to the confirmation page.

Yup, the confirmation page is the 2nd modal

Got it! Changes look good!

@FergusMok FergusMok requested a review from moziliar April 9, 2022 07:06
@FergusMok FergusMok added s.ToReview The PR is waiting for review(s) and removed s.Ongoing The PR is being worked on by the author(s) labels Apr 9, 2022
jayasting98
jayasting98 approved these changes Apr 9, 2022
View reviewed changes
Copy link
Contributor

@jayasting98 jayasting98 left a comment

Choose a reason for hiding this comment

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

Seems fine to me.

@moziliar
Copy link
Contributor

I'm largely good with this PR. Will take a final scan later today before approving!

samuelfangjw
samuelfangjw approved these changes Apr 10, 2022
View reviewed changes
Copy link
Member

@samuelfangjw samuelfangjw left a comment

Choose a reason for hiding this comment

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

Looks good to me!

moziliar
moziliar approved these changes Apr 11, 2022
View reviewed changes
Copy link
Contributor

@moziliar moziliar left a comment

Choose a reason for hiding this comment

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

LGTM!

@samuelfangjw samuelfangjw added the s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging label Apr 11, 2022
@samuelfangjw samuelfangjw removed the s.ToReview The PR is waiting for review(s) label Apr 11, 2022
@samuelfangjw samuelfangjw merged commit bda123f into TEAMMATES:master Apr 11, 2022
@FergusMok
Copy link
Contributor Author

Thanks everyone for taking the time to review this

ziqing26 pushed a commit to ziqing26/teammates that referenced this pull request Apr 11, 2022
@FergusMok @halfwhole
@jayasting98 @samuelfangjw @ziqing26
[TEAMMATES#11571] Selective Deadline Extensions - Frontend tables and…
c515928 
… modals (TEAMMATES#11603)

* [TEAMMATES#11571] Add selective-deadline-extension to CI workflow (TEAMMATES#11573)

* [TEAMMATES#11571] Selective Deadline Extensions - C_UD selective deadlines (TEAMMATES#11649)

* save progress

* Add modal

* Add mockups

* Add extension deadline table

* Add checkbox ability

* Add table highlighting ability

* Add radio button functionality

* Refactor modal logic

* Refactor time logic, add button validation

* Add time functionality for dropdown

* Add date modal

* Add callback from extension modal

* Add confirm modal

* Add labelling to customize options

* Remove unintended changes

* Fix linting errors

* Add snapshot tests and refactor

* Update failing snapshot tests

* Remove linting errors

* Add component tests

* Fix component tests

* Add instructors and endpoints

* Update snapshot tests

* Add loading-retry to tables

* Update tests

* Fix linting errors

* Fix PR comments

* Remove highlighting for instructors table

* Fix failing test

* Fix  timeZoneService inst undefined error

* Fix table sorting; Remove institute field; Add role field

* Fix linting errors

* Fix role comparison

* Change default sorting to end date

* Change small test to normal size

* Shift text and change extend button

* Change delete button validation

* Change initial sorting

* Fix rebase regressions

* Fix PR comments

* Add parallel loading of ccourses and feedback session

* Change outline color of course and session details

* Fix linting errors and update tests

* Shift extend and delete buttons to bottom right

* Format all files

* Fix side effects in pipe

* Reset tables after eaching loading

* Fix phrasing and dates of modals

* Add loading status upon submission

* Add validation to date modal; Fix date  modal tests; Make click applicable to row;

* Destructure students and instructors in function

* Refactor model mapping

* Add crosses to error messages

* Add row hover

* Fix failing test

* Remove stray file from rebase

* Fix 12 hours issue

* Add DateTime enumm for date modal

* Restructure guard clauses; Use object destructuring

* Shift isLoading into finalize

* Refactor update and delete

* Add comment to formatting date

* Fix radio enum rebase errors

* Revert wrong indentation

* Fix addAndFormatTime naming

* Fix formatting errors

* Remove e2e tests

* Add PR review changes

* Add time-related tests

* Add break line to getExtensionTimestamp

* Change or statement for hasSelected functions

Co-authored-by: halfwhole <[email protected]>
Co-authored-by: Jay Ting <[email protected]>
Co-authored-by: Samuel Fang <[email protected]>
@wkurniawan07 wkurniawan07 added the c.Feature User-facing feature; can be new feature or enhancement to existing feature label Apr 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@damithc damithc damithc left review comments

@jayasting98 jayasting98 jayasting98 approved these changes

@samuelfangjw samuelfangjw samuelfangjw approved these changes

@halfwhole halfwhole halfwhole approved these changes

@moziliar moziliar moziliar approved these changes

Assignees

@damithc damithc

@moziliar moziliar

@halfwhole halfwhole

@samuelfangjw samuelfangjw

@jayasting98 jayasting98

Labels
c.Feature User-facing feature; can be new feature or enhancement to existing feature s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging
Projects
None yet
Milestone
V8.14.0
Development

Successfully merging this pull request may close these issues.
7 participants
@FergusMok @moziliar @samuelfangjw @damithc @halfwhole @jayasting98 @wkurniawan07