Opened 9 years ago

Closed 9 years ago

#6051 closed enhancement (fixed)

Close TeamMonitor after restarting desktop

Reported by: humdinger Owned by: korli
Priority: normal Milestone: R1
Component: Servers/input_server Version: R1/Development
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: yes Platform: All

Description

I'd like you to consider a small change in TeamMonitor's behaviour.
Most of the time, when I invoke TM to kill or restart Tracker/Deskbar, that's the only reason I started TM. That means that I won't need it any longer and it would be OK if TM closes after clicking "Restart Desktop".
Also, if Tracker/Deskbar isn't running, it makes sense to make the "Restart Desktop" button the default.

Put together, restarting the desktop would be much quicker: ALT+CTRL+DEL then RETURN.

I attached a little 2 line patch for that. Is it OK if I apply it or are there arguments against this change?

Attachments (1)

TeamMonitor.diff (2.7 KB) - added by humdinger 9 years ago.
updated patch

Download all attachments as: .zip

Change History (10)

comment:1 Changed 9 years ago by humdinger

Has a Patch: set

comment:3 Changed 9 years ago by axeld

Sounds good, please go ahead.

comment:4 Changed 9 years ago by stippi

I am not so sure. Why optimize for a special case that is not supposed to happen in the first place? I am not fond of optimizing a workflow that a "regular user" is supposed to do only very rarely. It has good potential of becomming irritating. Changing the default button makes sense, though.

comment:5 Changed 9 years ago by humdinger

Rethinking it, I tend to agree with stippi. The TM window closing when choosing "Restart Desktop" is not really expected behaviour from a button. So, since we now have B_QUIT_ON_ESCAPE (or what it's called exactly...) I'm also OK with that. Even though, if the regular user doesn't have to do it, the button wouldn't appear in the first place... :)
Anyway, I'll apply the MakeDefault patch later.

WRT the Team Monitor: Does it have to be a modal window? Isn't an "always on top" feel enough?

comment:6 Changed 9 years ago by axeld

As you said, the regular user wouldn't see/notice/know that window, anyway, and all the other buttons in that row close the window somehow. I think it would actually be more consistent to make it close the window.

One could set the default button to "Cancel" if restarting the desktop isn't shown.

comment:7 Changed 9 years ago by humdinger

I still think not automatically closing on "Restart Desktop" is more consistent. Contrary to the buttons "Force Reboot" and "Cancel", "Restart Desktop" doesn't imply that TM will be quit.

Anyway, what's more important right now is that a straight forward change of simply adding a SetDefaultButton() seems to mess up the layout: maybe the "Default"-frame around the button isn't considered by the layout manager?
I'd love to play around with it some more, but after jamming and exchanging /boot/system/add-ons/input_server/device/keyboard the system drops into gdb and I have to reboot. I tried quitting the input_server first and restarting it after exchanging the file, but then it still drops into gdb when I hit a key...

This makes working on it extremely cumbersome. Is there a trick?

comment:8 Changed 9 years ago by humdinger

I attached a new patch making "Cancel" the default if "Restart Desktop" is hidden. I had to move the line "fRestartButton->Hide()" further down when building the GUI. Otherwise, the Cancel button would move to the right when the "Restart Desktop" button is finally shown. I'm not sure why that is...

It now behaves generally how it should. There's one thing however that I don't understand: Invoke TM, kill Deskbar, cancel TM, invoke TM again, press "Restart Desktop". Instead of the button being hidden again after launching Deskbar, it's just disabled. If I replace the responsible line "fRestartButton->SetEnabled(!desktopRunning);" with a "fRestartButton->Hide();" the button is never shown...

Maybe someone can improve the patch?

Changed 9 years ago by humdinger

Attachment: TeamMonitor.diff added

updated patch

comment:9 Changed 9 years ago by humdinger

If I replace the responsible line "fRestartButton->SetEnabled(!desktopRunning);" with a "fRestartButton->Hide();" the button is never shown...

Forgot: the hiding of the fRestartButton is inside a "if (desktopRunning)".

comment:10 Changed 9 years ago by humdinger

Resolution: fixed
Status: newclosed

I decided to commit my changes so far. It solves what has been discussed in this ticket and I've seen that the small issues outlined in my above comments also occur with the original code... So, fixed for now with hrev37063.

Note: See TracTickets for help on using tickets.