Opened 10 years ago

Closed 8 years ago

#4880 closed enhancement (fixed)

Auto-hide Deskbar feature [ autohide: on/off ]

Reported by: streak Owned by: axeld
Priority: normal Milestone: R1
Component: Applications/Deskbar Version: R1/alpha1
Keywords: Cc: superstippi@…
Blocked By: Blocking:
Has a Patch: yes Platform: All

Description

I think it's very important feature to auto-hide deskbar if deskbar is not focused. We can preserve a lot of screenspace with this.

Nice to have an option in deskbar properties to auto-hide deskbar.

Attachments (4)

DesbarAutoHideFeature.diff (12.1 KB ) - added by x-ist 8 years ago.
DesbarAutoHideFeature.2.diff (12.1 KB ) - added by x-ist 8 years ago.
DeskBarAutoHideFeature.diff (13.4 KB ) - added by x-ist 8 years ago.
Now correctly handling the AlwaysOnTop as well.
DeskbarAutoHideFeature.diff (13.3 KB ) - added by x-ist 8 years ago.
Style cleanup and readded B_AVOID_FRONT flag to TBarWindow again.

Download all attachments as: .zip

Change History (29)

comment:1 by jackburton, 10 years ago

Indeed, would be nice.

comment:2 by stpere, 10 years ago

How would it look like when hidden? Completely hidden? or with some border showing where it is? I suppose moving the mouse to that border would make it appear back?

comment:3 by streak, 10 years ago

Exactly, like you said "stpere" "moving the mouse to small border would make it appear back"

by x-ist, 8 years ago

Attachment: DesbarAutoHideFeature.diff added

comment:4 by x-ist, 8 years ago

Has a Patch: set

by x-ist, 8 years ago

comment:5 by x-ist, 8 years ago

This is a patch providing the Auto-Hide feature plus a modification to be discussed:

  1. Auto-Hide works with all possible deskbar positions, i.e. along the screen edge and in the corners. On the edges it shrinks to a thin line, whereas in the corners it shrinks to a dot. Probably there are other ideas how it would look better. Maybe two thin lines "deskbar borders" meeting in the corner.
  1. I modified the Auto-Raise behaviour not to activate the deskbar but rather to bringt it temporarily to top only. I found it simply really annoying to loose the focus from an editor while having a look at the deskbar with the mouse cursor.
  1. The whole deskbar source code does not follow the haiku coding style. So the patch integrates into what's there. I thought for patch review it would be better to see the functional changes first. Code style cleanup could then be applied too.

comment:6 by x-ist, 8 years ago

Sorry for the duplicate file upload - it's just the same.

... and what I forgot to mention. Dragging the deskbar using CTRL+CMD + mouse works again with the patch. Although I don't understand why it didn't with B_MOUSE_DOWN message in DispathMessage of BWindow.

comment:7 by x-ist, 8 years ago

The previous patch didn't take the AlwaysOnTop feature into account. This should be fixed now, so here is another patch.

by x-ist, 8 years ago

Attachment: DeskBarAutoHideFeature.diff added

Now correctly handling the AlwaysOnTop as well.

in reply to:  5 ; comment:9 by jscipione, 8 years ago

  1. The whole deskbar source code does not follow the haiku coding style. So the patch integrates into what's there. I thought for patch review it would be better to see the functional changes first. Code style cleanup could then be applied too.

I did some work a little while back to clean up the Deskbar style so that it would comply to the guidelines. Is there a particular part of the source code or file that you are seeing a lot of coding style violations in?

comment:10 by jscipione, 8 years ago

I applied your patch (DeskBarAutoHideFeature.diff) and it does seem to work well. I look forward to this patch being included in the project, unfortunately I have no authority to do so. You got rid of the overloaded TBarWindow::DispatchMessage() from BarWindow.cpp which should never have been there in the first place, congrats. I have only 2 small nitpicks. Firstly I would change the case kAutoRaise: in BarApp.cpp from

    case kAutoRaise:
        if (fSettings.alwaysOnTop)
            fSettings.autoRaise = false;
        else
            fSettings.autoRaise = !fSettings.autoRaise;
        ...

to

    case kAutoRaise:
        fSettings.autoRaise = fSettings.alwaysOnTop ? false :
            !fSettings.autoRaise;
        ...

Also could you create the patch from the root (haiku) directory instead of the deskbar directory. You can include only changes from that directory in your patch by issuing:

svn diff src/apps/deskbar > DeskBarAutoHideFeature.diff

from the haiku dir. Also, could you please mark your old patches as obsolete by checking the obsolete flag at the bottom of the patch file. Thank you for your hard work.

Last edited 8 years ago by jscipione (previous) (diff)

comment:11 by jscipione, 8 years ago

Also in case kAlwaysTop: in BarApp.cpp you added the line:

fPreferencesWindow->PostMessage(kStateChanged);

but that doesn't seem to actually do anything i.e. it doesn't make the deskbar raise on top of other windows so I think the line is superfluous.

comment:12 by x-ist, 8 years ago

Has a Patch: unset

in reply to:  11 comment:13 by x-ist, 8 years ago

Replying to jscipione:

Also in case kAlwaysTop: in BarApp.cpp you added the line:

fPreferencesWindow->PostMessage(kStateChanged);

but that doesn't seem to actually do anything i.e. it doesn't make the deskbar raise on top of other windows so I think the line is superfluous.

It's purpose is to trigger the _EnableDisableDependentItems method, which then enables/disables the AutoRaise checkbox depending on the Always on top checkbox state.

Your other comments are obeyed :)

comment:14 by x-ist, 8 years ago

Has a Patch: set

in reply to:  9 comment:15 by x-ist, 8 years ago

Replying to jscipione:

  1. The whole deskbar source code does not follow the haiku coding style. So the patch integrates into what's there. I thought for patch review it would be better to see the functional changes first. Code style cleanup could then be applied too.

I did some work a little while back to clean up the Deskbar style so that it would comply to the guidelines. Is there a particular part of the source code or file that you are seeing a lot of coding style violations in?

I would say most if not all of the header files don't comply with the coding style.

comment:16 by axeld, 8 years ago

I've looked at the patch, and it looks good for the most part (there are a few style violations left, though, more in a few lines).

However, why did you remove the B_AVOID_FRONT flag from the Deskbar? It was there for a reason, and should not be removed. If the Deskbar can become the front window, it means that floating windows of an application will be gone each time you click on the Deskbar.

A few minor coding style issues:

  • If there is only a single statement in an if-clause, you should not use curly braces, as they are just extraneous.
  • Line 206 of src/apps/deskbar/BarView.cpp in your diff should be indented one more tab, as the || belongs to another logical level than the && on the next line. That makes nested clauses easier to read.
  • Introducing variables like isHidden that you only use once should be omitted if there is not much logic to retrieve its value (like calling IsHidden() :-)) - that just causes more code to look at without simplifying it. isTopMost is okay, for example, as that property isn't that obvious from an all-floating window.

BTW when you write new code, it should always follow the Haiku coding style in files that are supposed to follow it -- no matter what's actually there; you'll just create more work for the one doing a follow-up change to fix the issues. This is just a remark, I didn't see anything particular in this regard.

in reply to:  16 comment:17 by x-ist, 8 years ago

Replying to axeld:

However, why did you remove the B_AVOID_FRONT flag from the Deskbar? It was there for a reason, and should not be removed. If the Deskbar can become the front window, it means that floating windows of an application will be gone each time you click on the Deskbar.

I added the flag again, however, doesn't than mean that B_AVOID_FRONT is equal to "avoid Always on Top feature"?

A few minor coding style issues:...

Should be gone now.

BTW when you write new code, it should always follow the Haiku coding style in files that are supposed to follow it -- no matter what's actually there; you'll just create more work for the one doing a follow-up change to fix the issues. This is just a remark, I didn't see anything particular in this regard.

Understood. What about the header files? I'd like to clean up their style as soon as this patch is accepted.

by x-ist, 8 years ago

Attachment: DeskbarAutoHideFeature.diff added

Style cleanup and readded B_AVOID_FRONT flag to TBarWindow again.

comment:18 by x-ist, 8 years ago

As I already read several times in the ML, here the "appreciated" small reminder: This patch should be ready for commit, isn't it? That would also close #1471.

comment:19 by axeld, 8 years ago

Status: newin-progress

Thanks, and sorry! I've already applied the patch locally, and tested it a bit. The only thing I did not like was that showing only works if the Deskbar is actually visible.

Anyway, I'll try to commit it later today as is, and thanks for your work!

comment:20 by jscipione, 8 years ago

Another feature I'd like to see integrated into this patch is that if you have the autohide setting turned on when you first boot it should show the Deskbar for ~5 seconds or so before hiding it.

comment:21 by stippi, 8 years ago

Cc: superstippi@… added

in reply to:  20 comment:22 by x-ist, 8 years ago

Replying to jscipione:

Another feature I'd like to see integrated into this patch is that if you have the autohide setting turned on when you first boot it should show the Deskbar for ~5 seconds or so before hiding it.

Indeed, that could be helpful. I will have a look at that tonight.

comment:23 by jscipione, 8 years ago

Showing the Deskbar on startup for a few seconds before hiding it should not hold up this patch from being included, another ticket can be created for this feature. I just wanted to make a note of it before I forget. Even if you don't get to this feature I believe this patch can be applied. If you do get to the feature that would be nice though.

in reply to:  23 comment:24 by x-ist, 8 years ago

Replying to jscipione:

Showing the Deskbar on startup for a few seconds before hiding it should not hold up this patch from being included, another ticket can be created for this feature.

That would be my preference too, as I don't know exactly when I will do it. Spare time is rare ATM.

comment:25 by axeld, 8 years ago

Resolution: fixed
Status: in-progressclosed

Thanks for you patch x-ist! I've finally applied it in hrev42383.

There are a couple of things that could be improved as outlined in the comments, plus a little hide/show animation would be nicer than doing that instant. I'm closing this ticket, anyway, though.

Note: See TracTickets for help on using tickets.