Closed Bug 952937 Opened 11 years ago Closed 11 years ago

Rocketbar: e.me Search Suggestions

Categories

(Firefox OS Graveyard :: Gaia, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: amirn, Assigned: amirn)

References

Details

Attachments

(2 files)

Show Everything.me auto-complete suggestions in the Rocketbar
Blocks: 948317
No longer blocks: 948317
Blocks: 948317
Attached file Redirect to PR (deleted) —
This patch includes a working version of Everything.me suggestions in the rocketbar
Attachment #8351186 - Flags: review?(ran)
Attachment #8351186 - Flags: review?(kgrandon)
(In reply to Amir Nissim (Everything.me) from comment #1) > Created attachment 8351186 [details] > Redirect to PR > > This patch includes a working version of Everything.me suggestions in the > rocketbar A better approach is to create a shared Evme object owning the connection, and have the Evme search and suggestion providers use it. When I tried approach everything stopped working. Kevin - can you please have a look at this patch? it looks like some weird IAC voodoo :) https://github.com/EverythingMe/gaia/commit/d9a7905c10bc6f7c44591e47eeee1d155c88a893
Flags: needinfo?(kgrandon)
Assignee: nobody → amirn
(In reply to Amir Nissim (Everything.me) from comment #2) > > A better approach is to create a shared Evme object owning the connection, > and have the Evme search and suggestion providers use it. > When I tried approach everything stopped working. It should be possible, but hard to say what was wrong without seeing a patch. I think something like this is probably fine for now though. I'll take a look today.
Flags: needinfo?(kgrandon)
Comment on attachment 8351186 [details] Redirect to PR Ok, looks mostly good to me. Before I give R+, had a few questions. 1 - Results appear to not be clearing for me currently. Sometimes there can be multiple levels of results. I think you need to clear them before you append more. 2 - Results are showing up for the same input currently. We should probably just not show results in this case. 3 - The search input is currently receiving the annotated query with brackets in the [q]uery. Should this be the case? 4 - Annotated examples will duplicate the exact search sometimes. E.g., if I search for 'britney spears', I will see a result, `[britney][spears]`. It seems like we should filter out exact matches, and that spaces should be considered in the annotated text.
Attachment #8351186 - Flags: review?(kgrandon)
Thanks Kevin. Just to make sure, did you see the patch at https://github.com/EverythingMe/gaia/commit/d9a7905c10bc6f7c44591e47eeee1d155c88a893 ? I am planning to change the implementation which will hopefully resolve some of the issues. (In reply to Kevin Grandon :kgrandon from comment #4) > 1 - Results appear to not be clearing for me currently. Sometimes there can > be multiple levels of results. I think you need to clear them before you > append more. Do you mean suggestions? it is cleared on every new search: https://github.com/EverythingMe/gaia/blob/8e22c1dc08e948d2cd64476ef37919204f641aab/apps/search/js/providers/eme_suggestions.js#L28 > 2 - Results are showing up for the same input currently. We should probably > just not show results in this case. I do not understand what do you mean by "same input". Can you please explain? > 3 - The search input is currently receiving the annotated query with > brackets in the [q]uery. Should this be the case? The annotation are received from E.me api but are cleaned before sent out to the search app https://github.com/EverythingMe/gaia/blob/8e22c1dc08e948d2cd64476ef37919204f641aab/apps/homescreen/everything.me/js/search/suggestion.js#L7 Where do you see the annotations? > 4 - Annotated examples will duplicate the exact search sometimes. E.g., if I > search for 'britney spears', I will see a result, `[britney][spears]`. It > seems like we should filter out exact matches, and that spaces should be > considered in the annotated text. The API returns the searched string as the 2nd suggestions. This is by design, but we can filter it out if needed.
(In reply to Amir Nissim (Everything.me) from comment #5) > Thanks Kevin. > > Just to make sure, did you see the patch at > https://github.com/EverythingMe/gaia/commit/ > d9a7905c10bc6f7c44591e47eeee1d155c88a893 ? > I am planning to change the implementation which will hopefully resolve some > of the issues. > > (In reply to Kevin Grandon :kgrandon from comment #4) > > 1 - Results appear to not be clearing for me currently. Sometimes there can > > be multiple levels of results. I think you need to clear them before you > > append more. > Do you mean suggestions? it is cleared on every new search: > https://github.com/EverythingMe/gaia/blob/ > 8e22c1dc08e948d2cd64476ef37919204f641aab/apps/search/js/providers/ > eme_suggestions.js#L28 You need to clear it before appending, or abort in-progress requests. Currently if I type three letters, and a request takes longer than our search delay, we end up showing multiple rows of suggestions. > > 2 - Results are showing up for the same input currently. We should probably > > just not show results in this case. > I do not understand what do you mean by "same input". Can you please explain? If I type 'omfgwtfbbq', the suggestion row shows a single suggestion of the same text. Seems like we should not have a suggestion row in this case. > > 3 - The search input is currently receiving the annotated query with > > brackets in the [q]uery. Should this be the case? > The annotation are received from E.me api but are cleaned before sent out to > the search app > https://github.com/EverythingMe/gaia/blob/ > 8e22c1dc08e948d2cd64476ef37919204f641aab/apps/homescreen/everything.me/js/ > search/suggestion.js#L7 > Where do you see the annotations? My query is updated after I click on a result with annotations. By annotation, I mean brackets surrounding the match terms. It doesn't seem like we should put the brackets into the input field. > > 4 - Annotated examples will duplicate the exact search sometimes. E.g., if I > > search for 'britney spears', I will see a result, `[britney][spears]`. It > > seems like we should filter out exact matches, and that spaces should be > > considered in the annotated text. > The API returns the searched string as the 2nd suggestions. This is by > design, but we can filter it out if needed. I see this first in some cases, and it seems wrong. We should probably filter this out. Please let me know if any of these don't make sense, I could upload a screenshot where necessary.
Blocks: 953058
updated PR with refactored code that introduces a global eme object owning the connection. RE [1]: Canceling requests is a provider-wide issue so IMO the search app should handle it. RE [3]: where do you see the annotations? Can you provide a screenshot? also attaching a screenshot from my device. RE issues [2] and [4]: This is how E.me API works (return the searched query as a one of the suggestions). IMO the eme client should not modify the API results. If we want to filter some suggestions out, I think a better place for that is in the search app. Thanks.
Attached image screenshot-no-annotations.png (deleted) —
Flags: needinfo?(kgrandon)
Comment on attachment 8351186 [details] Redirect to PR Marking R+ for the search app part of things. It's not perfect, but we can make follow-up patches no problem. It's early enough that we can iterate quickly, and we should do so.
Attachment #8351186 - Flags: review+
Flags: needinfo?(kgrandon)
Comment on attachment 8351186 [details] Redirect to PR Got a few comments on Github. Rock on.
Attachment #8351186 - Flags: review?(ran) → review+
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Depends on: 953121
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: