Closed
Bug 515747
Opened 15 years ago
Closed 15 years ago
bookmark manager is confusing to know if you are managing or not
Categories
(Firefox for Android Graveyard :: Bookmarks, defect, P1)
Firefox for Android Graveyard
Bookmarks
Tracking
(fennec1.0+)
VERIFIED
FIXED
fennec1.0b5
| Tracking | Status | |
|---|---|---|
| fennec | 1.0+ | --- |
People
(Reporter: jmaher, Assigned: vingtetun)
References
Details
(Whiteboard: [polish])
Attachments
(1 file, 2 obsolete files)
|
(deleted),
patch
|
mfinkle
:
review+
|
Details | Diff | Splinter Review |
when clicking "manage" in the bookmark manager, I need visual indication that I am editing bookmarks. Also the "manage" text is really small and hard to read.
Please:
1) make the manage text larger to match similar title bar font sizes
2) have a pressed state that toggled when it is pressed
|
Assignee | |
Comment 1•15 years ago
|
||
22:02 <@mfinkle> jmaher: file a bug on the manage being pressed
22:02 <@mfinkle> and one for making the first row be in edit mode
22:02 < jmaher> first row in edit mode?
22:03 <@mfinkle> jmaher: madhava suggested it
|
||
Comment 2•15 years ago
|
||
I'm not completely sod on the above design. The user still has no idea how to edit other rows. My initial thought was a small banner across the top say "Click a row to edit the bookmark". Once the user clicked any row the banner would hide.
|
Reporter | |
Comment 3•15 years ago
|
||
I really like the simplicity, I don't know how intuitive it really is. Maybe make all rows have the open edit feature and when you tap on it to edit it opens up the other fields (url, tags) and buttons?
|
|
||
Comment 6•15 years ago
|
||
Madhava Enros, from bug 517551:
Right now, when you press the "Manage" button in the "All Bookmarks"
list/manager, it remains visually inset, so you know that you've done
_something_, but there is no other visual cue that anything has changed or how
to proceed.
I think the simplest thing we could do to show people how to proceed would be
to pop open one of the rows to show its editable nature. If the one we open
isn't the one that the user was looking to edit, it should be easier to make
the leap to tapping on the one that they want. We could open up the first
visible row on the screen - other thoughts?
|
|
||
Updated•15 years ago
|
tracking-fennec: --- → ?
|
||
Updated•15 years ago
|
Whiteboard: [polish]
|
|
Assignee | |
Comment 7•15 years ago
|
||
* first row go into managed mode when we hit the 'Manage' button
* disable the manage button is there is nothing to manage (or nothing more)
Assignee: nobody → 21
|
|
||
Comment 8•15 years ago
|
||
Comment on attachment 402591 [details] [diff] [review]
wip
>diff -r 867a2dac0998 chrome/content/bindings.xml
> <setter>
> <![CDATA[
> if (this._manageUI != val) {
> this._manageUI = val;
> if (this._manageUI) {
> this.setAttribute("ui", "manage");
>+ this._children.firstChild.startEditing();
Not sure I like it here. Why not add it on the "Manage" button click?
> if (aFolder)
> query.setFolders([aFolder], 1);
> let result = PlacesUtils.history.executeQuery(query, options);
> let rootNode = result.root;
> rootNode.containerOpen = true;
>+
>+ let button = document.getElementById("tool-bookmarks-manage");
>+ if (aFolder == PlacesUtils.bookmarks.unfiledBookmarksFolder && rootNode.childCount == 0) {
>+ button.disabled = true;
>+ return items;
>+ }
>+ button.disabled = false;
>+
Add this to the BookmarkList.show method
> <method name="removeItem">
> <parameter name="aItem"/>
> <body>
> <![CDATA[
> this._children.removeChild(aItem);
>+
>+ let isRootFolder = (this._parents.lastChild.getAttribute("itemid") == PlacesUtils.bookmarks.unfiledBookmarksFolder);
>+ if (isRootFolder && this._children.childNodes.length == 0) {
>+ this.manageUI = false;
>+
>+ let button = document.getElementById("tool-bookmarks-manage");
>+ button.checked = false;
>+ button.disabled = true;
>+ }
Hmm, "remove" method currently fires a "BookmarkRemove" event. You could make BookmarkList listen for the event. That would keep all of this code outside of the XBL.
Attachment #402591 -
Flags: review-
|
|
Assignee | |
Comment 9•15 years ago
|
||
Address most of the comments.
(In reply to comment #8)
> (From update of attachment 402591 [details] [diff] [review])
> >diff -r 867a2dac0998 chrome/content/bindings.xml
>
> > <setter>
> > <
|
||
Updated•15 years ago
|
tracking-fennec: ? → 1.0+
|
|
Assignee | |
Comment 11•15 years ago
|
||
(In reply to comment #10)
> the .stopEditing call does not really need a balance. In this case, it's there
> to cleanup the editing mode if the manageUI is turned off while still in edit
> mode.
>
> If you want to keep the .startEditing code in this setter, then add a new
> attribute (autoedit="true") to the XUL and check for it in manageUI setter. If
> the attribute is "true", you can start editing.
>
> Sound ok?
Sounds perfect!
Attachment #406143 -
Attachment is obsolete: true
Attachment #406360 -
Flags: review?(mark.finkle)
Attachment #406143 -
Flags: review?(mark.finkle)
|
|
||
Comment 12•15 years ago
|
||
Comment on attachment 406360 [details] [diff] [review]
Patch v0.2
Works great. Thanks.
Attachment #406360 -
Flags: review?(mark.finkle) → review+
|
|
||
Comment 13•15 years ago
|
||
We should add a test to one of the browser_bookmark files for this
|
|
||
Comment 14•15 years ago
|
||
Status: NEW → RESOLVED
Closed: 15 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → B5
|
||
Comment 15•15 years ago
|
||
We'll need to update our litmus test case for this as well.
verified FIXED on builds:
Mozilla/5.0 (Windows; U; WindowsCE 5.2; en-US; rv:1.9.2b1pre) Gecko/20091021 Fennec/1.0a4pre
and
Mozilla/5.0 (X11; U; Linux armv7l; en-US; rv:1.9.2b1pre) Gecko/20091021
Fennec/1.0b5pre
and
Mozilla/5.0 (X11; U; Linux armv6l; en-US; rv:1.9.3a1pre) Gecko/20091021
Fennec/1.0b5pre
Status: RESOLVED → VERIFIED
Flags: in-litmus?
|
||
Comment 16•15 years ago
|
||
Updated litmus case: https://litmus.mozilla.org/show_test.cgi?searchType=by_id&id=7640
Flags: in-litmus? → in-litmus+
|
|
||
Updated•15 years ago
|
Component: General → Bookmarks
|
|
||
Updated•15 years ago
|
Flags: in-testsuite?
You need to log in
before you can comment on or make changes to this bug.
Description
•