Closed
Bug 1016239
Opened 10 years ago
Closed 10 years ago
[Collections App] Do not show web results that are pinned to Collection (dedupe)
Categories
(Firefox OS Graveyard :: Gaia, defect)
Tracking
(feature-b2g:2.0, tracking-b2g:backlog, b2g-v2.0 fixed)
RESOLVED
FIXED
2.0 S3 (6june)
Tracking | Status | |
---|---|---|
b2g-v2.0 | --- | fixed |
People
(Reporter: amirn, Assigned: kgrandon)
References
Details
(Whiteboard: [systemsfe][p=3])
Attachments
(2 files)
No description provided.
Reporter | ||
Updated•10 years ago
|
Blocks: collection-app
Depends on: 1016238
Updated•10 years ago
|
blocking-b2g: --- → backlog
feature-b2g: --- → 2.0
Whiteboard: [systemsfe]
Assignee | ||
Comment 1•10 years ago
|
||
I would like to abstract the dedeupe logic from the search app in order to handle this.
Assignee | ||
Comment 2•10 years ago
|
||
I did the dedupe logic in the search app, and would like to extract and use it here. Taking.
Assignee: nobody → kgrandon
Status: NEW → ASSIGNED
Whiteboard: [systemsfe] → [systemsfe][p=3]
Target Milestone: --- → 2.0 S3 (6june)
Assignee | ||
Comment 3•10 years ago
|
||
Hey guys - could you give this a quick review? This should make it easy to actually use this across apps. Thanks!
Attachment #8430226 -
Flags: review?(ran)
Attachment #8430226 -
Flags: review?(amirn)
Reporter | ||
Comment 4•10 years ago
|
||
Comment on attachment 8430226 [details]
Part 1 - Abstract dedupe.js from search.js
Kevin, what do you think about making deduping a gaia-grid feature and extract the files to /shared?
Since it will also be used in Collections I think the term GridDedupe (instead of SearchDedupe) is more appropriate.
We can do that later of course.
Attachment #8430226 -
Flags: review?(amirn) → review+
Updated•10 years ago
|
Attachment #8430226 -
Flags: review?(ran) → review+
Comment 5•10 years ago
|
||
Amir's suggestion speaks to me.
Assignee | ||
Comment 6•10 years ago
|
||
Sounds like a good idea to me. This has a dependency on bug 1017222 though. The only reason I might be adverse to it is if we would ever find the need to use it outside of a "grid" view. Hopefully not though.
Assignee | ||
Comment 7•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #8430226 -
Attachment description: Github pull request → Part 1 - Abstract dedupe.js from search.js
Assignee | ||
Comment 8•10 years ago
|
||
Hey Guys - this commit contains the work I did in the other bug for displaying of pinned apps, as well as a working dedupe implementation. Can you take a look? Thanks!
Attachment #8433615 -
Flags: review?(ran)
Attachment #8433615 -
Flags: review?(amirn)
Reporter | ||
Comment 9•10 years ago
|
||
Comment on attachment 8433615 [details]
Part 2 - Implement deduping of web results from pinned results
added comments on Github.
Thanks.
Attachment #8433615 -
Flags: review?(amirn) → review+
Comment 10•10 years ago
|
||
Comment on attachment 8433615 [details]
Part 2 - Implement deduping of web results from pinned results
Added comments on Github as well
Attachment #8433615 -
Flags: review?(ran) → review+
Assignee | ||
Comment 11•10 years ago
|
||
Ok, went ahead and landed this: https://github.com/mozilla-b2g/gaia/commit/4506564ed5cb37dc007c83241e3e6f2a7671179e
I think the main concern is whether or not dedupe should be moved into the grid logic. I'm still not convinced, but we can do this in a follow-up if you guys feel strongly about it.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 12•10 years ago
|
||
Landed a follow-up, missed the dedupe.js path in the search app, oops: https://github.com/mozilla-b2g/gaia/commit/8f94479062343666d5fc1eb29af7e3dec4b2c4a7
Will work on a marionette test for this.
Updated•10 years ago
|
status-b2g-v2.0:
--- → fixed
Updated•10 years ago
|
blocking-b2g: backlog → ---
tracking-b2g:
--- → backlog
You need to log in
before you can comment on or make changes to this bug.
Description
•