695199 - Bookmarking: Add/Remove from Menu

Status: VERIFIED FIXED

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

Description

[Erin Lancaster [:elan]]

* need UI design

Comment 1

[Aaron Train [:aaronmt]]

This including Add/Remove and *Edit*?

Comment 2

[Erin Lancaster [:elan]]

Aaron: Product and Eng have decided no Edit Functionality at this point.

Comment 3

[Sriram Ramasubramanian [:sriram]]

Attachment: 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.

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

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?

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

Note: ICS will use images in the action bar (ifRoom), just not sure about the dropdown menu

Comment 6

[Sriram Ramasubramanian [:sriram]]

Attachment: 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.

Comment 7

[Sriram Ramasubramanian [:sriram]]

Attachment: Patch

Refreshed patch.

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

pushed rebased patch
https://hg.mozilla.org/projects/birch/rev/97403e114b58

Comment 9

[Aaron Train [:aaronmt]]

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."

Comment 10

[Mark Finkle (:mfinkle) (use needinfo?)]

(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

Comment 11

[Aaron Train [:aaronmt]]

Mozilla/5.0 (Android; Linux armv7l; rv:10.0a1) Gecko/20111027 Firefox/10.0a1 Fennec/10.0a1