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

[#8931] Instructor: Feedback Session: Mark mandatory fields for creating and editing session #12617

Merged
merged 5 commits into from
Nov 30, 2023
Merged

[#8931] Instructor: Feedback Session: Mark mandatory fields for creating and editing session #12617

merged 5 commits into from
Nov 30, 2023

Conversation

Ting-TT
Copy link
Contributor

@Ting-TT Ting-TT commented Oct 22, 2023

Fixes #8931

Mark "Session Name" as a mandatory field during new session creation in the following ways:

  1. Append a * symbol after "Session Name" when creating a new session. (Note: When editing a session, the session name cannot be modified, so the "Session Name" label remains unchanged and does not display the * symbol.)
  2. Disable the "Create Feedback Session" button when the "Session Name" field is empty.
  3. Display a red warning with an exclamation mark, stating "The field Session Name should not be empty," when the instructor clicks the field but leaves it empty.

(I modelled changes 2 and 3 based on the behaviour of the "Course Name" field during new course creation.)


Screenshots

Before the changes:

teammates_session_name_before

After the changes:
Below is a screenshot illustrating the updates. The areas circled in red encompass all the modifications I introduced.

teammates_session_name_mandatory

@Ting-TT
Copy link
Contributor Author

Ting-TT commented Oct 22, 2023

Ready for review :))

@cedricongjh cedricongjh self-requested a review October 22, 2023 19:05
@cedricongjh cedricongjh self-assigned this Oct 22, 2023
cedricongjh
cedricongjh requested changes Oct 22, 2023
View reviewed changes
Copy link
Contributor

@cedricongjh cedricongjh left a comment

Choose a reason for hiding this comment

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

hey @Ting-TT thank you for the contribution!

looks fine to me, let's also add this to the copy from previous feedback sessions modal to keep the behaviour consistent:
image

@cedricongjh cedricongjh added the s.ToReview The PR is waiting for review(s) label Oct 22, 2023
@Ting-TT
Copy link
Contributor Author

Ting-TT commented Oct 24, 2023

Thank you for pointing that out!
I've implemented the following changes to ensure consistency between the two methods of creating a new feedback session:

Mark the "Name for new session" field mandatory when creating by copying from a previous session:

  1. Append a * symbol after "Name for new session"
  2. Display a red warning with an exclamation mark, stating "The field 'Name for new session' should not be empty," when the instructor clicks the field but leaves it empty.

Screenshots

Before the changes:

Name_for_new_session_before.

After the changes:

Name_for_new_session_after

@Ting-TT
Copy link
Contributor Author

Ting-TT commented Oct 24, 2023

Ready for review :))

cedricongjh
cedricongjh approved these changes Oct 25, 2023
View reviewed changes
Copy link
Contributor

@cedricongjh cedricongjh left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for your contribution to TEAMMATES!

@cedricongjh cedricongjh added s.FinalReview The PR is ready for final review and removed s.ToReview The PR is waiting for review(s) labels Oct 25, 2023
@Ting-TT
Copy link
Contributor Author

Ting-TT commented Oct 25, 2023

No worries! Thank you for reviewing :))

@nusoss-bot
Copy link

Folks, This PR seems to be stalling (no activities for the past 7 days). 🐌 😢
Hope someone can get it to move forward again soon...

2 similar comments
@nusoss-bot
Copy link

Folks, This PR seems to be stalling (no activities for the past 7 days). 🐌 😢
Hope someone can get it to move forward again soon...

@nusoss-bot
Copy link

Folks, This PR seems to be stalling (no activities for the past 7 days). 🐌 😢
Hope someone can get it to move forward again soon...

@nusoss-bot
Copy link

Folks, This PR seems to be stalling (no activities for the past 11 days). 🐌 😢
Hope someone can get it to move forward again soon...

weiquu
weiquu approved these changes Nov 30, 2023
View reviewed changes
Copy link
Contributor

@weiquu weiquu left a comment

Choose a reason for hiding this comment

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

LGTM

@weiquu weiquu removed the request for review from jasonqiu212 November 30, 2023 09:12
@weiquu weiquu added s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging and removed s.FinalReview The PR is ready for final review labels Nov 30, 2023
@weiquu weiquu merged commit f2edb0a into TEAMMATES:master Nov 30, 2023
9 checks passed
@cedricongjh cedricongjh mentioned this pull request Dec 11, 2023
Closed
@weiquu weiquu mentioned this pull request Dec 20, 2023
Merged
@cedricongjh cedricongjh mentioned this pull request Jan 8, 2024
Open
@wkurniawan07 wkurniawan07 added the c.Feature User-facing feature; can be new feature or enhancement to existing feature label Jan 21, 2024
@wkurniawan07 wkurniawan07 added this to the V8.30.0 milestone Jan 21, 2024
@leojiangxd leojiangxd mentioned this pull request Jan 25, 2024
Closed
cedricongjh pushed a commit to cedricongjh/teammates that referenced this pull request Feb 20, 2024
@Ting-TT @weiquu @cedricongjh
[TEAMMATES#8931] Instructor: Feedback Session: Mark mandatory fields …
73adae2 
…for creating and editing session (TEAMMATES#12617)

* mark the session name field mandatory when add a new feedback seesion

* Fix linting issues

* Update tests related with my changes by modifying snapshot

* Mark the 'Name for new session' field mandatory when create by copying

---------

Co-authored-by: Wei Qing <48304907+weiquu@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cedricongjh cedricongjh cedricongjh approved these changes

@weiquu weiquu weiquu approved these changes

Assignees

@cedricongjh cedricongjh

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.30.0
Development

Successfully merging this pull request may close these issues.

Instructor: Feedback Session: Mark mandatory fields for creating and editing session
5 participants
@Ting-TT @nusoss-bot @weiquu @cedricongjh @wkurniawan07