Skip to content
This repository has been archived by the owner on Sep 7, 2020. It is now read-only.

Piazza search (WiP) #126

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Piazza search (WiP) #126

wants to merge 7 commits into from

Conversation

Sumukh
Copy link
Member

@Sumukh Sumukh commented Mar 22, 2017

Not ready for prime time but I would like to try this out in some 61A office hours

Known issues:

  • On first load, the search ignores the description
  • This probably should be it's own component.
  • How do we disable this for other courses?

let user = encodeURIComponent(this.props.state.currentUser.hash)

if (description) {
query += ' ' + description;
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be sanitized

@knrafto
Copy link
Member

knrafto commented Mar 22, 2017

An idea on how to configure:

  • Algolia search URL is set as a environment variable in deployment (in the future, a course-specific config option)
  • In the state message (which happens on page load / reconnect), send the URL to the client
  • If no URL is set, Piazza search is disabled

That way the client doesn't need to change between courses.

@knrafto
Copy link
Member

knrafto commented Mar 22, 2017

I think the popup after clicking a feedback button is pretty annoying. You could use addMessage to flash a message at the top instead.

@Sumukh
Copy link
Member Author

Sumukh commented Mar 22, 2017

Thanks - going to try this out in the 3-4pm office hours. I'll be there to observe (or rollback if needed)

@Sumukh
Copy link
Member Author

Sumukh commented Mar 22, 2017

I realized that this causes the buttons to jump around for the staff view. We should place the piazza search results below the buttons

@Sumukh
Copy link
Member Author

Sumukh commented Apr 19, 2017

Ran the experiment and found that it didn't have enough usage to be interesting. It's been removed from production.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants