Make sending articles to Reading List easier from home panel add-ons

RESOLVED WONTFIX

Status

()

Firefox for Android
Awesomescreen
P2
normal
RESOLVED WONTFIX
4 years ago
2 years ago

People

(Reporter: kar, Assigned: rricard, Mentored)

Tracking

({uiwanted})

Trunk
All
Android
uiwanted
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(fennec+)

Details

Attachments

(5 attachments)

(Reporter)

Description

4 years ago
Because we'd like to sync the reading list over to other Firefox instances attached to FxA (including over to desktop), I'd like to make it even easier to use the Reading List as a repository of articles you'd like to read later (or on a different screen), offline or online.

There as a start, can we add 'send to reading list' [or something along those lines] to the long-press menu when I tap on an item in another home panel feed I have installed? Alternate ways also welcome.
Any UX feedback?
tracking-fennec: ? → +
Flags: needinfo?(ywang)
Flags: needinfo?(alam)
I think that's a good idea. Our quick win is to add "Add to Reading List" in the long press menu for articles. It's the same wording as the action for the Share overlay.
Flags: needinfo?(ywang)
Created attachment 8488259 [details]
prev_rmode_menu1.png

That sounds like a good starting point.

If we have this goal in mind of working with our desktop counterparts, maybe we could even put that in the right click menu as well? Not unlike our attempts at trying to do something like this (which was more technically challenging).
Flags: needinfo?(alam)
Created attachment 8488260 [details]
Screen Shot 2014-09-11 at 2.43.01 PM.png

Attaching desktops right click menu.
(Reporter)

Comment 5

4 years ago
If it would be in the sharing element, we would need to refer to this as Reading List and not reader mode - because the idea is we're adding it to the list (plus I *hate* having 'mode' in-product)
(In reply to Anthony Lam (:antlam) from comment #3)
> Created attachment 8488259 [details]
> prev_rmode_menu1.png
> 
> That sounds like a good starting point.
> 
> If we have this goal in mind of working with our desktop counterparts, maybe
> we could even put that in the right click menu as well? Not unlike our
> attempts at trying to do something like this (which was more technically
> challenging).

We already have this in the Sharing system. If you share using "Firefox", "Add to Reading List" is already there.

This bug is 100% about the Home Panels inside Firefox.
Created attachment 8496088 [details]
Screenshot_2014-09-26-10-38-30.png

Fair enough, an item in the long press menu makes sense when the user is in the Panels view.

Is that what the "Add to Home Screen" does? I pressed it but not sure where it "added" the link to... 

If not, "Add to Reading list" makes sense :) Maybe a toast too?
Flags: needinfo?(krudnitski)
(Reporter)

Comment 8

4 years ago
'Add to Home Screen' looks like it ads the link directly to my Android home screen as a short cut. This is awful :(. Can we *not* do that and replace that item to 'Add to Reading List'?
Flags: needinfo?(krudnitski)
(In reply to Karen Rudnitski [:kar] from comment #8)
> 'Add to Home Screen' looks like it ads the link directly to my Android home
> screen as a short cut. This is awful :(. Can we *not* do that and replace
> that item to 'Add to Reading List'?

I'm not exactly sure what it does. I found that in Nightly when I long pressed on a link in my History Panel.
(Reporter)

Comment 10

4 years ago
I'm not sure what you're saying here. Right now, 'add to home screen' means the link gets pasted on my wallpaper as a link. 

Now I do know that my husband uses that feature *all* the time as a way to bookmark his top sites directly from his wallpaper. Can we check telemetry to see how many people are like my husband and simply add to the list in order to 'add to reading list'?
filter on [mass-p5]
Priority: -- → P5
(Reporter)

Comment 12

4 years ago
this is core to future work, so we need to ensure that we make it as simple and easy for folks to add articles to their reading list.
tracking-fennec: + → ?
Priority: P5 → --
Mentor: margaret.leibovic
tracking-fennec: ? → +
Keywords: uiwanted
Priority: -- → P2
(Assignee)

Comment 13

3 years ago
This one should be nice to work on. I think I'll take it after fixing bug 1130872.
(Assignee)

Comment 14

3 years ago
Maybe add some tests too ...
(Assignee)

Updated

3 years ago
Assignee: nobody → ricard.robin
Status: NEW → ASSIGNED
(Assignee)

Comment 15

3 years ago
I'll need some help to work on it though ...
(Assignee)

Comment 16

3 years ago
Created attachment 8564572 [details] [diff] [review]
Make sending articles to Reading List easier from home panel add-ons

Add a new menu item
Bind the menu item to a Reading List Addition
Attachment #8564572 - Flags: review?(margaret.leibovic)
(Assignee)

Comment 18

3 years ago
Created attachment 8564576 [details]
Feature added, screenshot
(Assignee)

Comment 19

3 years ago
Uh ! bad try flags !

Comment 21

3 years ago
Thanks for picking this up! I think we need to get some input here from antlam about what we want to be doing here.

antlam, do we want an "Add to Reading List" context menu item on all home panel items?
Flags: needinfo?(alam)

Comment 22

3 years ago
Comment on attachment 8564572 [details] [diff] [review]
Make sending articles to Reading List easier from home panel add-ons

Review of attachment 8564572 [details] [diff] [review]:
-----------------------------------------------------------------

This is going in the right direction, I just want to get some feedback from antlam before proceeding further.

::: mobile/android/base/home/HomeFragment.java
@@ +260,5 @@
> +
> +            AddToReadingList addToReadingList = new AddToReadingList(context);
> +            ShareData shareData = new ShareData(info.url, info.title, null, ShareMethod.Type.ADD_TO_READING_LIST);
> +            addToReadingList.handle(shareData);
> +            return true;

This is an interesting way to re-purpose the share overlay code. However, I think it would be more straightforward to just call into BrowserDB directly, like we do else where in this class. You'll also want to do this in a background task, since it's doing disk I/O.

I also think you should wait until the patches bug 1130461 land, since those will change the APIs available to you (and I think it should make this easier).
Attachment #8564572 - Flags: review?(margaret.leibovic) → feedback+

Comment 23

3 years ago
Comment on attachment 8564572 [details] [diff] [review]
Make sending articles to Reading List easier from home panel add-ons

Review of attachment 8564572 [details] [diff] [review]:
-----------------------------------------------------------------

rnewman, what do you think of this share handler approach as opposed to just directly querying BrowserDB?
Attachment #8564572 - Flags: feedback?(rnewman)
(In reply to :Margaret Leibovic from comment #21)
> Thanks for picking this up! I think we need to get some input here from
> antlam about what we want to be doing here.
> 
> antlam, do we want an "Add to Reading List" context menu item on all home
> panel items?

Yeah I think that's still a pretty valid idea. We might have to unify on copy here like "Add to Reading List" instead of "Send to Reading List" but I like where this is headed :) thanks Rricard!
Flags: needinfo?(alam)
Comment on attachment 8564572 [details] [diff] [review]
Make sending articles to Reading List easier from home panel add-ons

Review of attachment 8564572 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/home/HomeFragment.java
@@ +260,5 @@
> +
> +            AddToReadingList addToReadingList = new AddToReadingList(context);
> +            ShareData shareData = new ShareData(info.url, info.title, null, ShareMethod.Type.ADD_TO_READING_LIST);
> +            addToReadingList.handle(shareData);
> +            return true;

Yeah, ReadingListAccessor will do pretty much exactly this without having to make a ShareData instance.
Attachment #8564572 - Flags: feedback?(rnewman) → feedback+
Depends on: 1130461
Version: Firefox 35 → Trunk
(Assignee)

Comment 26

3 years ago
Ok, going to work on ReadingListAccessor instead. Thanks for the feedback !
(Assignee)

Comment 27

3 years ago
I'll wait for bug 1130461 first, ping me if you need help on it. I'm following it anyway.
Flags: needinfo?(margaret.leibovic)

Comment 28

3 years ago
(In reply to Robin Ricard [rricard] from comment #27)
> I'll wait for bug 1130461 first, ping me if you need help on it. I'm
> following it anyway.

Okay, thanks.

My one concern is that this change requires a string, so it would be good to land that before the merge. We have a bit of leeway on reader-related string for Fx38 aurora, but we could also just pre-land the strings for this. If you're interested in doing that, could you split up your patch to create an initial patch that just adds the string for the context menu? I think it should be "Add to Reading List" for consistency with the other places we have this in our UI.
Flags: needinfo?(margaret.leibovic)

Comment 29

2 years ago
We're removing the reading list in bug 1234314.
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.