Opened 11 years ago

Closed 5 years ago

#3385 closed bug (fixed)

Open Disks Icon with Single Window Navigation = no draggable folder icon in upper right

Reported by: mmadia Owned by: aldeck
Priority: normal Milestone: R1
Component: Applications/Tracker Version: R1/pre-alpha1
Keywords: Cc: mattmadia@…
Blocked By: Blocking:
Has a Patch: no Platform: All

Description

This only occurs when the Disks Icon and Single Window Navigation is enabled : Any Tracker browser window that is opened by double-clicking the Disks icon will not have the handy-dandy draggable folder icon in the upper-right corner.

Note: if I right-click the disks-icon and navigate to a sub-folder, the icon appears.

Attachments (1)

trackersinglewindownavpatch.diff (11.2 KB ) - added by dru_ed 9 years ago.
This patch fixes all bugs mentioned in Ticket.

Download all attachments as: .zip

Change History (8)

comment:1 by mmadia, 11 years ago

Cc: mattmadia@… added

All these issues originate when starting a Tracker Browser Window by double clicking the Disks Icon on Desktop. As such, I'm including them in this ticket. If these should be created as separate tickets, let me know and I'll change it.

First notice the lack of the handy-dandy dragger folder icon in the upper left of the menu bar. Any sub-target browsed to by left clicking will also experience this. From now on, "sub-target" will refer to such a window.

Now, from a sub-target press Alt+(up_arrow_key). In single window navigation mode, the current window should simply display its parent. However, in a sub-target, a new Tracker window will appear. This new Tracker window will have the correct behavior for Alt+(up_arrow_key) and the handy-dandy dragger folder icon.

Next, in a sub-target, press Alt+N. A new folder is not created.

These issues do not occur if you right-click the disks icon and then left-click a drive or any folder beneath a drive

by dru_ed, 9 years ago

This patch fixes all bugs mentioned in Ticket.

comment:2 by dru_ed, 9 years ago

I've submitted a patch that fixes a host of Single Window Mode related bugs including those mentioned in the Ticket.

  • Draggable folder icons should appear where expected. Trash & Disks do not have them - this appeared to be the desired behavior and works appropriately. If they should have icons (perhaps "dimmed" in appearance), that can be done.
  • Alt+(up_arrow_key) no longer incorrectly creates a new window as mentioned in the Ticket case.
  • Alt+N appears to work correctly and where appropriate now.
  • Disks ("/" aka Root) File menu has be corrected to only show items that are appropriate to its Volume contents.

Let me know if you encounter bugs/crashes or unexpected behavior.

dru345 on #haiku/#haiku_dev

comment:3 by aldeck, 9 years ago

Owner: changed from axeld to aldeck
Status: newassigned

Thanks! I'll have a look soon, though i'm about to commit big changes in tracker in the comming days (regarding layout kit and containerwindow / poseview ties). Your patch will need to be "ported" to the future code, but in the mean time, you will need to improve the patch anyway (see below).

I'd like you to explain more in detail the changes you did, i.e: why the previous code was buggy and how you changed it.

If you'd like to leave commented out code (that's rarely needed) you should explain clearly why in the code itself (or update an existing comment to reflect a new reality).

Concerning style, a few blank lines were unnecessarily added/removed and some spaces at line endings as well as bad spacing before brackets. You create a local variable fMenuBar, names starting with "f" are reserved for class member variables.

Best regards.

comment:4 by dru_ed, 9 years ago

Thanks for the feedback. I'll look at the style issues you mentioned.

Generally, most changes are to address the specific case of starting or moving into "Disks" (Root, "/") while in spatial mode and often the fix is as "simple" as saying "user's in Root", treat it as a special case just like "Trash."

Addressing these bugs was really about looking at the behavior in fine detail and doing touchups in the right spots rather than large rewrites although a rewrite/refactoring of Tracker may be highly desirable long term.

Regarding the menu changes, I observed if you opened the "Disks" (Root, "/") window, the "File" menu contents were different than if you traveled up to Root ("/") from a child container.

Typical contents of the File menu are: Find…, Open, Get info, Edit name, Duplicate, Move to Trash, Move to, Copy to, Create link, Cut, Copy, Paste, Identify, Add-ons. Only a few of these make sense in context of Disks, (Root, "/") and indeed when the opened at the Disks icon, the File menu contents were more appropriate. Unfortunately, that wasn't always so depending on how the user reached the location.

Investigating, I found difference in Disks' (Root, "/") File menu content, depending on how you arrived (dbl-click icon or traveling up by Navigator), was due to a conflict between the conceptual model of Spatial & Single Window Mode. In existing code, even the order of the menu items varied depending on how the user arrived at the same destination.

Spatial mode opened the Disks (Root, "/") window, as a subclass of ContainerWindow, the VolumeWindow, while other containers used only the parent class. I couldn't determine a safe or reliable method to dynamically cast the the existing navigator Window from parent window type to the child window type and vice versa as needed when traversing, accordingly I moved the few lines of effected menu code from child to the parent and added a few conditionals in the existing code. Appropriate menus now display regardless of whether a user arrived by the icon on Desktop or via the "Up" arrow on the Navigator toolbar.

The ticket item regarding a new window opening when the user travels to parent in Single Window Mode was caused by sole reliance upon the "Open Parent" Spatial code while missing a case where Single Window Mode expects to reuse the same window. This was fixed by adding code to address use of the "Open Parent" (ALT+up_arrow_key) menu item while also in Single Window Mode by sending a BMessage to the window. It was not a bug with the "Up" toolbar icon because it had not reused the "Open Parent" Spatial code.

Regarding the missing menu bar icons, the original code missed cases specific to Root ("/") / Disks when in Navigator mode. The solution was to delete the draggable icon in select cases like Trash & Root to match desired behavior. The icon cache is used outside these cases for move containers.

I hope this helps explain the hows and whys regarding my patch. Thanks for the comments on style. With regards to commented out code, that was an oversight. I should've removed it before diffing the patch. I look forward to "porting" it to your updated code.

I should note there are still things to address including determining out the "correct" or desired conceptual model for Trash and observed inconsistent use of keyboard modifiers.

At present, as with the old code, Trash is shown as existing at /boot/trash. This is, of course, not strictly true since it's really a metafolder with contents from across each /{volume name}/trash directories. At present a user can traverse "Up" from Trash which currently means to /boot even though a user opened the Trash icon on the Desktop and the Trash icon is not presented visually as a Symlink.

Questions arise: should Trash be treated as existing in "Trash" alone and not offering a particular path, should a user be able to open the "parent" of Trash and what would that be: /boot?, /{volume name}, ~/ Desktop, or some other location ?

Regarding inconsistent use of keyboard modifiers, I observed use of Option (Windows key) while clicking the "Up" arrow on the Navigator Toolbar results in a different behavior than using Option + "Open Parent" menu item (or its keyboard shortcut). Both open the Parent in a new window. However, the Navigator Toolbar method keeps the child window open while the conceptually equivalent menu item, "Open Parent" closes the child window when used with the Option key modifier.

So far, I've charted the 36 cases: Single Window Mode (Icon vs. menu item), Spatial (menu item) each for no modifier, ctrl, cmd (alt), option (win) across these questions: opens the parent? opens a new window? keeps the child window open?

Use of the menu item (and its keyboard shortcut) are largely consistent between Spatial & Single Window Mode. It's the Navigator toolbar which breaks the mold. The issue is still under investigation.

comment:5 by diver, 9 years ago

Is this patch also takes care about #5584?

in reply to:  5 comment:6 by dru_ed, 9 years ago

Replying to diver:

Is this patch also takes care about #5584?

No, I didn't know about the #5584 issue at the time I did this patch. The context menu is handled by separate code but the underlying issue is the same. Since aldeck is doing a rewrite of parts of Tracker involved in #3385, #5584 (and maybe other tickets) I think this patch will ultimately be made redundant.

If parts of it are still needed, it'll need to be reworked in light of the Tracker rewrites.

Last edited 9 years ago by dru_ed (previous) (diff)

comment:7 by stpere, 5 years ago

Resolution: fixed
Status: assignedclosed

Should be fixed in hrev47498.

Thanks!

Note: See TracTickets for help on using tickets.