Closed Bug 1312467 Opened 8 years ago Closed 8 years ago

Add Activity Stream context menu support to TopSites

Categories

(Firefox for Android Graveyard :: Awesomescreen, defect, P1)

Product:
Firefox for Android Graveyard
Component:
Awesomescreen
Type:
defect

Tracking

(firefox52 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 52
Tracking Flags:
Tracking Status
firefox52 --- fixed

People

(Reporter: ahunt, Assigned: ahunt)

References

Details

(Whiteboard: [MobileAS])

Attachments

(6 files)

Bug 1312467 - Move touch target size definition into resources
58 bytes, text/x-review-board-request
sebastian
: review+
Details
Bug 1312467 - Move TouchTarget handling into generic helper class
58 bytes, text/x-review-board-request
sebastian
: review+
Details
Bug 1312467 - Also store topsites title in TopSite data structure
58 bytes, text/x-review-board-request
sebastian
: review+
Details
Bug 1312467 - Pass url-open listeners into TopSitesCard and handle clicks there
58 bytes, text/x-review-board-request
sebastian
: review+
Details
Bug 1312467 - Add context menu support to topsites
58 bytes, text/x-review-board-request
sebastian
: review+
Details
Bug 1312467 - Hide some items for TopSites context menu
58 bytes, text/x-review-board-request
sebastian
: review+
Details 
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: as-android-newtab
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
Assignee: nobody → ahunt
Status: NEW → ASSIGNED
Iteration: --- → 1.7
Priority: -- → P1
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 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 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 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 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 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+
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
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: