Opened 6 years ago

Closed 5 years ago

#9534 closed bug (fixed)

[ActivityMonitor] Settings can't be opened if Always on top is enabled

Reported by: diver Owned by: leavengood
Priority: normal Milestone: R1
Component: Applications/ActivityMonitor Version: R1/Development
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: yes Platform: All

Description

See hrev45344.

Attachments (4)

0001-activity-monitor-settings.patch (2.2 KB) - added by Freeman 5 years ago.
0001-activity-monitor-settings.2.patch (2.1 KB) - added by Freeman 5 years ago.
0001-activitymonitor-settings.patch (2.2 KB) - added by Freeman 5 years ago.
0001-activitymonitor-settings.2.patch (1.8 KB) - added by Freeman 5 years ago.

Download all attachments as: .zip

Change History (19)

Changed 5 years ago by Freeman

comment:1 Changed 5 years ago by Freeman

Has a Patch: set

comment:2 Changed 5 years ago by dsjonny

(This is the same in MediaPlayer.)

comment:3 Changed 5 years ago by korli

Freeman, thanks for the patch. Here are my remarks:

  • I'd just add a method IsAlwaysOnTop() in ActivityWindow.h instead of using a constructor parameter.
  • fAlwaysOnTop is not a boolean but a pointer to a BMenuItem.
  • method or function parameter names begin with a lowercase character.

comment:4 Changed 5 years ago by Freeman

Do you mean to use the function as the parameter? Like new SettingWindow(this, _IsAlwaysOnTop());

comment:5 in reply to:  4 ; Changed 5 years ago by korli

Replying to Freeman:

Do you mean to use the function as the parameter? Like new SettingWindow(this, _IsAlwaysOnTop());

No, I mean that IsAlwaysOnTop() can be called in the SettingsWindow constructor.

comment:6 in reply to:  5 ; Changed 5 years ago by Freeman

Replying to korli:

Replying to Freeman:

Do you mean to use the function as the parameter? Like new SettingWindow(this, _IsAlwaysOnTop());

No, I mean that IsAlwaysOnTop() can be called in the SettingsWindow constructor.

How is the method in ActivityWindow going to be called by the constructor of SettingsWindow?

comment:7 in reply to:  6 Changed 5 years ago by korli

Replying to Freeman:

How is the method in ActivityWindow going to be called by the constructor of SettingsWindow?

target->IsAlwaysOnTop()

Changed 5 years ago by Freeman

comment:8 Changed 5 years ago by korli

The build fails with this patch applied.

comment:9 Changed 5 years ago by Freeman

Yeah I think I messed up my source by using reset --hard. Hold on I'll fix the code.

Changed 5 years ago by Freeman

comment:10 Changed 5 years ago by korli

Your patch includes an unneeded modification of the line 41/42 of ActivityWindow.h. Please fix. Also ActivityWindow::IsAlwaysOnTop() should be const.

Changed 5 years ago by Freeman

comment:11 Changed 5 years ago by korli

On a second thought, the feel shouldn't be changed if the ActivityWindow isn't always on top. Or do you have a good reason for that?

comment:12 Changed 5 years ago by Freeman

So what should be done at this point?

comment:13 Changed 5 years ago by korli

Resolution: fixed
Status: newclosed

Edited and applied in hrev46476.

comment:14 Changed 5 years ago by diver

Resolution: fixed
Status: closedreopened

Something is still not right.

  • Settings->Settings
  • Settings->Always on top

After that you can't open Settings window anymore.

comment:15 Changed 5 years ago by korli

Resolution: fixed
Status: reopenedclosed

Fixed in hrev46477. Thanks for testing!

BTW having both a settings window and a settings menu for a simple app seems over the top.

Note: See TracTickets for help on using tickets.