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

[#12532] Instructor Home Page: Remind dropdown expands into the table #12543

Merged
merged 12 commits into from
Aug 9, 2023
Merged

[#12532] Instructor Home Page: Remind dropdown expands into the table #12543

merged 12 commits into from
Aug 9, 2023

Conversation

rexong
Copy link
Contributor

@rexong rexong commented Aug 1, 2023

Fixes #12532

Outline of Solution
I referred to the dropdown box for Results and notice that the Remind dropdown's <div> does not have a container that is set to "body".
After adding container="body", the dropdown expands outside the table.

TEAMMATES.-.Online.Peer.Feedback_Evaluation.System.for.Student.Team.Projects.-.Google.Chrome.2023-08-01.16-53-36.mp4

@github-actions
Copy link

github-actions bot commented Aug 1, 2023

Hi @rexong, thank you for your interest in contributing to TEAMMATES!
However, your PR does not appear to follow our contribution guidelines:

  • Title must start with the issue number the PR is fixing in square brackets, e.g. [#<issue-number>]

Please address the above before we proceed to review your PR.

@rexong rexong changed the title Prevent dropdown to open inside table [#12532] Instructor Home Page: Remind dropdown expands into the table Aug 1, 2023
@weiquu
Copy link
Contributor

weiquu commented Aug 2, 2023

Hi @rexong, behaviour looks fine now but do take a look at the failing E2E test. In particular, it seems to be failing because the dropdown is now outside the courseTab element. You can take a look at the other methods in InstructorHomePage.java to see how this is being handled (e.g. in publishSessionResults())

@rexong
Copy link
Contributor Author

rexong commented Aug 6, 2023

Ready for Review!

weiquu
weiquu approved these changes Aug 8, 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 added the s.FinalReview The PR is ready for final review label Aug 8, 2023
jasonqiu212
jasonqiu212 reviewed Aug 8, 2023
View reviewed changes
Copy link
Contributor

@jasonqiu212 jasonqiu212 left a comment

Choose a reason for hiding this comment

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

Hi @rexong, One small question from me. The rest LGTM!

src/e2e/java/teammates/e2e/cases/InstructorHomePageE2ETest.java Outdated Show resolved Hide resolved
jasonqiu212
jasonqiu212 approved these changes Aug 8, 2023
View reviewed changes
Copy link
Contributor

@jasonqiu212 jasonqiu212 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 merged commit 8633d4e into TEAMMATES:master Aug 9, 2023
8 checks passed
@samuelfangjw samuelfangjw added s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging c.Bug Bug/defect report and removed s.FinalReview The PR is ready for final review labels Aug 21, 2023
@samuelfangjw samuelfangjw added this to the V8.29.0 milestone Aug 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weiquu weiquu weiquu approved these changes

@jasonqiu212 jasonqiu212 jasonqiu212 approved these changes

@EuniceSim142 EuniceSim142 Awaiting requested review from EuniceSim142

Assignees

@jasonqiu212 jasonqiu212

@weiquu weiquu

Labels
c.Bug Bug/defect report s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging
Projects
None yet
Milestone
V8.29.0
Development

Successfully merging this pull request may close these issues.

Instructor Home Page: Remind dropdown expands into the table
4 participants
@rexong @weiquu @jasonqiu212 @samuelfangjw