Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Bookmarking: Add/Remove from Menu

VERIFIED FIXED

Status

()

Firefox for Android
General
P1
normal
VERIFIED FIXED
6 years ago
a year ago

People

(Reporter: elan, Assigned: sriram)

Tracking

unspecified
ARM
Android
Points:
---

Firefox Tracking Flags

(firefox11 fixed, fennec11+)

Details

(Whiteboard: [birch] [ux needed] [Product Approve])

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

6 years ago
* need UI design

Updated

6 years ago
Assignee: nobody → lucasr.at.mozilla
Assignee: lucasr.at.mozilla → kgupta
(Reporter)

Updated

6 years ago
Summary: Bookmarking → Bookmarking: Add/List/Remove
(Reporter)

Updated

6 years ago
Whiteboard: [birch] [ux needed]
This including Add/Remove and *Edit*?
(Reporter)

Comment 2

6 years ago
Aaron: Product and Eng have decided no Edit Functionality at this point.
Whiteboard: [birch] [ux needed] → [birch] [ux needed] [Product Approve]
(Assignee)

Comment 3

6 years ago
Created attachment 569563 [details] [diff] [review]
WIP

This patch does add/remove bookmarks through the options menu.
When gecko is loading, since the URL is not known, both the options are hidden.
Once gecko loads and URL is known to Tabs, add or remove option is shown based on the presence/absence of bookmark.

Todo:
Edit option is currently not implemented. This needs UI spec.
Assignee: kgupta → sriram
Attachment #569563 - Flags: review?(mark.finkle)
Comment on attachment 569563 [details] [diff] [review]
WIP

* Do we need to use two menu items? Can we dynamically change the image state?
* Original designs only want to use "Bookmark" as the text, and the image would change depending on the state. I'm not sure if we can do that on ICS (menus with images). This patch uses "Add Bookmark" / "Remove Bookmark"

Can you rebase the patch so I can try it out?
Note: ICS will use images in the action bar (ifRoom), just not sure about the dropdown menu
(Assignee)

Comment 6

6 years ago
Created attachment 569770 [details] [diff] [review]
Patch

This patch adds just one entry in menu and changes it based on whether a bookmark is already added or not.
A toast message is shown based on add/remove action.

Todo:
Finalizing the string for the toast.
Replacing the icon assets.
Attachment #569563 - Attachment is obsolete: true
Attachment #569563 - Flags: review?(mark.finkle)
Attachment #569770 - Flags: review?(mark.finkle)
Attachment #569770 - Flags: review?(mark.finkle) → review+
(Assignee)

Comment 7

6 years ago
Created attachment 569803 [details] [diff] [review]
Patch

Refreshed patch.
pushed rebased patch
https://hg.mozilla.org/projects/birch/rev/97403e114b58
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Summary: Bookmarking: Add/List/Remove → Bookmarking: Add/Remove from Menu
I think for future l10n translations, the strings added in the patch above should remove the exclamation point.

"Bookmark added successfully!" -> "Saved to bookmarks."
"Bookmark removed!" -> "Removed bookmark." or "Bookmark removed."
(In reply to Aaron Train [:aaronmt] from comment #9)
> I think for future l10n translations, the strings added in the patch above
> should remove the exclamation point.
> 
> "Bookmark added successfully!" -> "Saved to bookmarks."
> "Bookmark removed!" -> "Removed bookmark." or "Bookmark removed."

Damn! I agree completely and I forgot to change those before landing. Just did a quick change:

https://hg.mozilla.org/projects/birch/rev/adad73e2ee6f
Mozilla/5.0 (Android; Linux armv7l; rv:10.0a1) Gecko/20111027 Firefox/10.0a1 Fennec/10.0a1
Status: RESOLVED → VERIFIED
tracking-fennec: --- → 11+
status-firefox11: --- → fixed
You need to log in before you can comment on or make changes to this bug.