Closed Bug 430213 Opened 15 years ago Closed 15 years ago

Fill details pane when folders in the left tree of the Library are selected

Categories

(Firefox :: Bookmarks & History, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 3

People

(Reporter: myron02, Assigned: mak)

References

Details

Attachments

(4 files, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008042115 Minefield/3.0pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008042115 Minefield/3.0pre

The properties editor is shown when a page is selected in the library's display area, but it isn't shown when folders in the left tree pane are selected. I think selecting folders in the pane should bring up the properties editor because it would make the process of renaming folders more intuitive compared to the current way (going to the parent folder and clicking on a sub folder). It would also decrease the number of clicks required to edit an item's properties.

Reproducible: Always

Steps to Reproduce:
1. Go to the "Bookmarks" menu
2. Click "Organize Bookmarks..."
3. Click a folder in the left tree pane
Actual Results:  
Only the number of items in the folder and some directions are shown in the display area.

Expected Results:  
The properties editor should be shown in the display area along with the number of items and the directions.
Attached image Proposed mockup
Version: unspecified → Trunk
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008042209 Minefield/3.0pre

Confirmed that no properties are shown in the bottom panel. I don't know if this is intended or just forgotten. 
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking-firefox3?
imo the details panel is right pane "devoted", but since now we don't have properties context in the left pane, this could alleviate the problem
(In reply to comment #4)
> imo the details panel is right pane "devoted", but since now we don't have
> properties context in the left pane, this could alleviate the problem
> 

What about placing the properties editor in a display area at the bottom? That way it won't be specific to the right or left panes. The editor then looks kind of large though, even if some text is moved around.
(In reply to comment #4)
> imo the details panel is right pane "devoted", but since now we don't have
> properties context in the left pane, this could alleviate the problem

I don't think that's really clear to the user, and it's confusing that selection in the left view will modify the top of the right view, but not the bottom of the right view.

We should fix this. Not a blocker, but wanted.

(Putting the properties across the entire bottom pane is probably a good idea, but not as part of this bug, and requires rethinking of the layout of the controls in that area.)
Flags: wanted-firefox3+
Flags: blocking-firefox3?
Flags: blocking-firefox3-
(In reply to comment #7)
> (Putting the properties across the entire bottom pane is probably a good idea,
> but not as part of this bug, and requires rethinking of the layout of the
> controls in that area.)

that would appear more correct, but will leave a lot of unused space in the properties panel (until we don't have the site preview, that could fill space well).
We should also consider that the screen there is taken with a few folders in the left pane but that could grow quite large, having less space in a tree could potentially make harder recognize parents / children of folders

I agree that this is surely wanted, but imo blocker if we don't reinsert properties context menu in the left pane items.
Summary: Show properties editor when folders in tree pane are selected → Show details pane when folders in the left tree of the Library are selected
Summary: Show details pane when folders in the left tree of the Library are selected → Fill details pane when folders in the left tree of the Library are selected
Yeah, I agree, either this or bug 430075 needs to block, and this is more discoverable.
Flags: wanted-firefox3+
Flags: blocking-firefox3-
Flags: blocking-firefox3+
Attached patch patch (obsolete) — Splinter Review
we should update the details pane in these cases:
- focus changes between the trees
- user clicks on an item
- selection changes

but we should also try avoiding update the details pane if we are already editing the same item.
Assignee: nobody → mak77
Status: NEW → ASSIGNED
Attachment #317553 - Flags: review?(mano)
OS: Windows XP → All
Hardware: PC → All
Whiteboard: [has patch][needs review mano]
mh, i did a mistake in the comment, we should update the details pane in these cases:
- focus changes between the trees
- selection changes in the left tree
- selection changes in the right tree
It looks like you no longer null check selectedNode... (which can happen, esp. in the right pane).

And please make it aSelectedNode.
there are the same checks as before on selectedNode

all three points are calling fillDetailsPane, that does a 
+      if (selectedNode && gEditItemOverlay.itemId == selectedNode.itemId &&

then after you have again      
     if (selectedNode && !PlacesUtils.nodeIsSeparator(selectedNode)) {

i have to call fillDetailsPane also if selectedNode is null
Comment on attachment 317553 [details] [diff] [review]
patch

yeah, i read that wrong, r=mano (please do s/selectedNode/aSelectedNode/).
Attachment #317553 - Flags: review?(mano) → review+
Whiteboard: [has patch][needs review mano] → [has patch][has reviews]
yep, will come with fixed local var, thank you
Attached patch patchSplinter Review
fixed local var name
Attachment #317553 - Attachment is obsolete: true
Attachment #317553 - Flags: approval1.9?
Comment on attachment 317553 [details] [diff] [review]
patch

a=mconnor on behalf of 1.9 drivers
Attachment #317553 - Flags: approval1.9? → approval1.9+
Keywords: checkin-needed
mozilla/browser/components/places/content/places.js 	1.161
mozilla/browser/components/places/content/places.xul 	1.133 
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [has patch][has reviews]
Target Milestone: --- → Firefox 3
verified with:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9pre) Gecko/2008042806 Minefield/3.0pre
Status: RESOLVED → VERIFIED
Depends on: 431140
No longer depends on: 431140
This has caused two regressions: bug 431528 and bug 430213
Depends on: 431140
Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in before you can comment on or make changes to this bug.