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

[#11126] Expand empty search results for feedback session logs #11151

Merged
merged 9 commits into from
Jun 5, 2021
Merged

[#11126] Expand empty search results for feedback session logs #11151

merged 9 commits into from
Jun 5, 2021

Conversation

hoonti06
Copy link
Contributor

@hoonti06 hoonti06 commented May 24, 2021 •
edited
Loading

Fixes #11126

Outline of Solution

  • Modified that tap expand status depends on whether feedback session log entry is empty or not

before
before

after
after

@moziliar moziliar added the s.ToReview The PR is waiting for review(s) label May 25, 2021
@moziliar
Copy link
Contributor

Hi @hoonti06 can you also attach the before-after snapshot for the ease of review?

@hoonti06
Copy link
Contributor Author

Hi @hoonti06 can you also attach the before-after snapshot for the ease of review?

Sure, I added gif files to compare before and after

moziliar
moziliar approved these changes May 26, 2021
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.

To me the change solves the problem.

Need @t-cheepeng to verify if there's any pitfall around this change.

t-cheepeng
t-cheepeng approved these changes May 27, 2021
View reviewed changes
Copy link
Contributor

@t-cheepeng t-cheepeng left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@t-cheepeng t-cheepeng added s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging and removed s.ToReview The PR is waiting for review(s) labels May 27, 2021
@madanalogy madanalogy added this to the V7.17.0 milestone May 28, 2021
@madanalogy madanalogy added the c.Feature User-facing feature; can be new feature or enhancement to existing feature label May 28, 2021
@moziliar
Copy link
Contributor

@hoonti06 the e2e test case is bind to the old UI. Please update the e2e test case

@madanalogy madanalogy added s.Ongoing The PR is being worked on by the author(s) and removed 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 labels May 28, 2021
@madanalogy madanalogy removed this from the V7.17.0 milestone May 28, 2021
@rrtheonlyone
Copy link
Contributor

Thanks for the work here @hoonti06 💯

Looks like E2E tests for Audit Logs are failing - could you take a look and see if you can fix? Let us know if you need any assistance on this.

@moziliar
Copy link
Contributor

Do try to run e2e test locally before pushing. It will save the time on remote CI and also allow us to get back to you faster, granted that we now limit the CI run for first PR to manual approval:)

@hoonti06
Copy link
Contributor Author

hoonti06 commented Jun 2, 2021

Sorry, I just succeeded in configuring my test environment locally.

rrtheonlyone
rrtheonlyone approved these changes Jun 5, 2021
View reviewed changes
Copy link
Contributor

@rrtheonlyone rrtheonlyone left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@rrtheonlyone rrtheonlyone added s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging and removed s.Ongoing The PR is being worked on by the author(s) labels Jun 5, 2021
@rrtheonlyone rrtheonlyone added this to the V7.17.0 milestone Jun 5, 2021
@rrtheonlyone rrtheonlyone merged commit 1123653 into TEAMMATES:master Jun 5, 2021
@madanalogy madanalogy added the c.Feature User-facing feature; can be new feature or enhancement to existing feature label Jun 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rrtheonlyone rrtheonlyone rrtheonlyone approved these changes

@moziliar moziliar moziliar approved these changes

@t-cheepeng t-cheepeng t-cheepeng approved these changes

Assignees

@rrtheonlyone rrtheonlyone

@moziliar moziliar

@t-cheepeng t-cheepeng

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
V7.17.0
Development

Successfully merging this pull request may close these issues.

Instructor checking logs: expand results if empty
5 participants
@hoonti06 @moziliar @rrtheonlyone @t-cheepeng @madanalogy