Add Activity Stream context menu support to TopSites

RESOLVED FIXED in Firefox 52

Status

()

P1
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: ahunt, Assigned: ahunt)

Tracking

Trunk
Firefox 52
Points:
---

Firefox Tracking Flags

(firefox52 fixed)

Details

(Whiteboard: [MobileAS])

Attachments

(6 attachments)

(Assignee)

Description

2 years ago
We can probably reuse mostly the same code.

Issues to figure out are:
- Pinning, do we want that? If yes:
-- Add pin/unpin menu items
-- Switch to using the pinned topsites query
- Rewire "Dismiss" to block from topsites and/or use the same blocklist for topsites (discuss behaviour with UX?)
Blocks: 1311099
Whiteboard: [MobileAS]
(In reply to Andrzej Hunt :ahunt from comment #0)
> - Pinning, do we want that? If yes:
> -- Add pin/unpin menu items
> -- Switch to using the pinned topsites query

For the first iteration we can ignore this.

The desktop team is currently exploring new ideas in regards to pinning (and "pinned" content will show up in highlights):
* https://trello.com/c/jqnypdPW/98-i-would-like-to-be-able-to-pin-content-and-set-reminders-for-content-on-the-new-tab-from-the-share-menu
* https://trello.com/c/dmLVRlmF/93-i-want-to-be-able-to-pin-down-a-useful-highlight-for-a-while-so-i-don-t-loose-it

However there's a trello card for pinning top sites for v1.1.:
https://trello.com/c/0cSeinDy/105-i-want-to-be-able-to-manually-edit-pin-the-top-sites
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Assignee: nobody → ahunt
Status: NEW → ASSIGNED
Iteration: --- → 1.7
Priority: -- → P1

Comment 8

2 years ago
mozreview-review
Comment on attachment 8804612 [details]
Bug 1312467 - Move touch target size definition into resources

https://reviewboard.mozilla.org/r/88528/#review88402
Attachment #8804612 - Flags: review?(s.kaspari) → review+

Comment 9

2 years ago
mozreview-review
Comment on attachment 8804613 [details]
Bug 1312467 - Move TouchTarget handling into generic helper class

https://reviewboard.mozilla.org/r/88530/#review88406

::: mobile/android/base/java/org/mozilla/gecko/util/TouchTargetUtil.java:5
(Diff revision 1)
> +/* -*- Mode: Java; c-basic-offset: 4; tab-width: 20; indent-tabs-mode: nil; -*-
> + * This Source Code Form is subject to the terms of the Mozilla Public
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +package org.mozilla.gecko.util;

nit: Most files have an empty line between license and package.

::: mobile/android/base/java/org/mozilla/gecko/util/TouchTargetUtil.java:14
(Diff revision 1)
> +import android.view.View;
> +
> +import org.mozilla.gecko.R;
> +
> +public class TouchTargetUtil {
> +

nit: Most classes start without an empty line.
Attachment #8804613 - Flags: review?(s.kaspari) → review+

Comment 10

2 years ago
mozreview-review
Comment on attachment 8804614 [details]
Bug 1312467 - Also store topsites title in TopSite data structure

https://reviewboard.mozilla.org/r/88532/#review88408
Attachment #8804614 - Flags: review?(s.kaspari) → review+

Comment 11

2 years ago
mozreview-review
Comment on attachment 8804615 [details]
Bug 1312467 - Pass url-open listeners into TopSitesCard and handle clicks there

https://reviewboard.mozilla.org/r/88534/#review88410

::: mobile/android/base/java/org/mozilla/gecko/home/activitystream/topsites/TopSitesPage.java:32
(Diff revision 1)
>      private HomePager.OnUrlOpenListener onUrlOpenListener = null;
> +    private HomePager.OnUrlOpenInBackgroundListener onUrlOpenInBackgroundListener = null;

nit: Class properties do not need to be initialized with null - they are null by default.
Attachment #8804615 - Flags: review?(s.kaspari) → review+

Comment 12

2 years ago
mozreview-review
Comment on attachment 8804616 [details]
Bug 1312467 - Add context menu support to topsites

https://reviewboard.mozilla.org/r/88536/#review88424
Attachment #8804616 - Flags: review?(s.kaspari) → review+

Comment 13

2 years ago
mozreview-review
Comment on attachment 8804617 [details]
Bug 1312467 - Hide some items for TopSites context menu

https://reviewboard.mozilla.org/r/88538/#review88426
Attachment #8804617 - Flags: review?(s.kaspari) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 25

2 years ago
Pushed by ahunt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b8c538f62798
Move touch target size definition into resources r=sebastian
https://hg.mozilla.org/integration/autoland/rev/330833c71a86
Move TouchTarget handling into generic helper class r=sebastian
https://hg.mozilla.org/integration/autoland/rev/a207a3a36465
Also store topsites title in TopSite data structure r=sebastian
https://hg.mozilla.org/integration/autoland/rev/aa12d5ecd8b6
Pass url-open listeners into TopSitesCard and handle clicks there r=sebastian
https://hg.mozilla.org/integration/autoland/rev/9f65ac29fa24
Add context menu support to topsites r=sebastian
https://hg.mozilla.org/integration/autoland/rev/a9f1a369aa95
Hide some items for TopSites context menu r=sebastian
Iteration: 1.7 → 1.8
You need to log in before you can comment on or make changes to this bug.