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.

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

[#11020] Remove searching response comments related features and search box at home page #11060

Merged
merged 3 commits into from
Mar 27, 2021

Conversation

wkurniawan07
Copy link
Member

First part of #11020

This removes all code related to searching response comments as well as the search box at the top of instructor home page.

This will also close #3918, close #8704, close #9103 since they are all irrelevant without the searching response comments feature.

@wkurniawan07 wkurniawan07 changed the title [#11020] [WIP] Remove searching response comments related features and search box at home page [#11020] Remove searching response comments related features and search box at home page Mar 23, 2021
@wkurniawan07 wkurniawan07 marked this pull request as ready for review March 23, 2021 12:38
@wkurniawan07 wkurniawan07 added the s.ToReview The PR is waiting for review(s) label Mar 23, 2021
@wkurniawan07 wkurniawan07 added this to the V7.13.0 milestone Mar 23, 2021
Copy link
Contributor

@Derek-Hardy Derek-Hardy left a comment

Choose a reason for hiding this comment

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

LGTM!

placeholder="e.g. Charles Shultz, charles@gmail.com">
<div class="input-group-append">
<button id="btn-search" class="btn btn-primary" (click)="search()">Search</button>
</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a good reason to remove the home search bar?

Understand it's just a redirection to the search page. From the UX perspective, this removal makes search operation become two-steps-away from login.

If search is a frequent action performed by the instructor, it will be convenient to make it immediately available at home page?

Copy link
Member Author

Choose a reason for hiding this comment

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

A search bar at home page heavily implies a site-wide search, which this search bar is nothing about. There are multiple cases of such mis-usage, like "how do I publish a session", "how do I enroll students", etc.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree that the search bar was a bit out of place on the homepage, esp. since we already have a search page. However, I still think that the current search bar is not explicit enough as to what is searchable and what is not. I believe we can address that in a new issue or after the search migration.

Copy link
Member Author

Choose a reason for hiding this comment

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

Screenshot 2021-03-26 at 6 45 30 PM

@Derek-Hardy Derek-Hardy self-assigned this Mar 25, 2021
Copy link
Contributor

@jianhandev jianhandev left a comment

Choose a reason for hiding this comment

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

There are still some mention of search comments feature in the following files:

  • features-page.component.html (line 212)
  • instructor-search-page.component.ts (line 42)

@wkurniawan07
Copy link
Member Author

@jianhandev I will merge after your approval

Copy link
Contributor

@jianhandev jianhandev left a comment

Choose a reason for hiding this comment

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

LGTM!

@jianhandev jianhandev 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 Mar 27, 2021
@wkurniawan07 wkurniawan07 merged commit be302ad into TEAMMATES:master Mar 27, 2021
@wkurniawan07 wkurniawan07 deleted the remove-search-frc branch March 27, 2021 05:45
@madanalogy madanalogy added the c.Feature User-facing feature; can be new feature or enhancement to existing feature label Apr 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
4 participants