Closed Bug 1144675 Opened 5 years ago Closed 5 years ago

The Reading List button from the Location Bar should have a distinct icon for pages currently in the list

Categories

(Firefox Graveyard :: Reading List, defect)

39 Branch
defect
Not set

Tracking

(firefox38 verified, firefox39 verified)

VERIFIED FIXED
Firefox 39
Tracking Status
firefox38 --- verified
firefox39 --- verified

People

(Reporter: avaida, Assigned: florian)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Reproducible on:
Nightly 39.0a1 (2015-03-18)

Affected platforms:
Ubuntu 14.04 (x64), Windows 7 (x64), Mac OS X 10.9.5.

Preconditions:
* browser.readinglist.enabled = true

Steps to reproduce:
1. Launch Firefox.
2. Access a web page, (e.g.) https://www.mozilla.org/en-US/mission/.
3. Click the "Add page to Reading List" button from the Location Bar.

Expected result:
* Clicking the button successfully adds the example page to Reading List.
* The button icon turns from (+) to (-) after it has been clicked.

Actual result:
The button is only changing its icon in terms of color, gray to blue. 
I think it would be more intuitive and also consistent with the design used in Reader View's sidebar controls to change this.
Flags: qe-verify+
Michael, the mockups imply this button is always a "+", with only the color changing - but clarkbw thought maybe a "-" was desired - what's your take?
Flags: needinfo?(mmaslaney)
The fact that hovering the "checked" icon makes it black, is quite confusing, since when hovering, you can't really see whether the page is in the reading list or not.
(In reply to Mark Hammond [:markh] from comment #1)
> Michael, the mockups imply this button is always a "+", with only the color
> changing - but clarkbw thought maybe a "-" was desired - what's your take?

Yes, we need it to change to a delete button like the reader view.  This should fix bug 1144684, might need to dupe that here.

10:08 AM <clarkbw> mmaslaney: the url bar reading list button is supposed change to a (-) minus after adding something, right?
10:08 AM <clarkbw> that's the behavior of the reader view add/remove button currently
10:08 AM <clarkbw> https://projects.invisionapp.com/share/B21NGU7M3#/screens/59725441?maintainScrollPosition=false
10:10 AM <mmaslaney> clarkbw: Yes

Michael, can you provide the delete button icon?
(In reply to Bryan Clark (Firefox PM) [:clarkbw] from comment #3)
> Michael, can you provide the delete button icon?

We are actually going to need it in our SVG - https://dxr.mozilla.org/mozilla-central/source/browser/themes/shared/readinglist/icons.svg - it may be easy for us to do it, but I'm not sure yet :/  If you know SVG, it would be great if you can tweak it.
Attached patch PatchSplinter Review
This patch:
- Changes the icon to a '-' when the page has already been added.
- Makes the icon blue only in the active state.
- Hides the icon when the URL bar is neither focused nor hovered.
- moves the readinglist.inc.css file to the readinglist subfolder.
Assignee: nobody → florian
Status: NEW → ASSIGNED
Flags: needinfo?(mmaslaney)
Attachment #8579848 - Flags: review?(mhammond)
Attachment #8579848 - Flags: review?(mhammond) → review+
Hi Florian, can you provide a point value.
Iteration: --- → 39.2 - 23 Mar
Flags: needinfo?(florian)
Flags: firefox-backlog+
Points: --- → 2
Flags: needinfo?(florian)
Blocks: 1132074
https://hg.mozilla.org/mozilla-central/rev/89cb4fd9ee1d
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 39
QA Contact: andrei.vaida
Verified fixed on Nightly 39.0a1 (2015-03-23), using Ubuntu 14.04 (x64), Mac OS X 10.9.5 and Windows 7 (x64). 

The Reading List button from the Location Bar now acts and looks as specified in Comment 5.
Status: RESOLVED → VERIFIED
Verified fixed on Aurora 38.0a2 (2015-03-29) as well, using Windows 7 (x64), Ubuntu 14.04 (x64) and Mac OS X 10.9.5.
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.