Opened 10 years ago

Closed 8 years ago

#3274 closed enhancement (fixed)

ScreenSaver: Improve presentation of disabled options because of missing DPMS support. (easy)

Reported by: HAL Owned by: axeld
Priority: low Milestone: R1
Component: Preferences/ScreenSaver Version: R1/pre-alpha1
Keywords: Cc:
Blocked By: Blocking: #3275
Has a Patch: no Platform: All

Description

If I open ScreenSaver and try to select a time for putting monitor into low power, there is no selection available "Turn off screen" is greyed out. I am using hrev28846 vmware image.

Attachments (2)

listdev (955 bytes) - added by HAL 10 years ago.
Shows graphics device
screensaver.png (76.0 KB) - added by vijay 10 years ago.

Download all attachments as: .zip

Change History (23)

Changed 10 years ago by HAL

Attachment: listdev added

Shows graphics device

comment:1 Changed 10 years ago by HAL

I accidentally duplicated this bug; something wrong with Firefox when I tried to upload attachment first time. It did not indicate that I had done so. I could not get any change on page by clicking again to upload. I clicked the back button on the browser for previous page where I was entering the bug and then with the Add attachment radio button selected, clicked the button to confirm and then add the attachment in second try. This time it did indicate on web page that it was uploaded but the bug was also duplicated. Please delete #3375.

comment:2 Changed 10 years ago by axeld

Resolution: invalid
Status: newclosed

The low power screen option cannot work in VESA mode.

comment:3 Changed 10 years ago by axeld

Blocking: 3275 added

comment:4 Changed 10 years ago by stippi

Priority: normallow
Resolution: invalid
Status: closedreopened
Summary: Unable to select "Turn off screen" in ScreenSaver preferences.ScreenSaver: Improve presentation of disabled options because of missing DPMS support.
Type: bugenhancement

We could however improve the GUI in the ScreenSaver preflet to make it obvious why the option is not available.

comment:5 Changed 10 years ago by stippi

Summary: ScreenSaver: Improve presentation of disabled options because of missing DPMS support.ScreenSaver: Improve presentation of disabled options because of missing DPMS support. (easy)

comment:6 Changed 10 years ago by vijay

Newbie developer here.Ryan suggested this ticket to me.Need some clarification on how the presentation can be improved?One way would be ,Instead of greying out the option we can enable the option, but when the user checks it we can have a dialog saying that the "DPMS support is unavailable".Any suggestions?

comment:7 Changed 10 years ago by stippi

First of all, you need to find a way to check the DPMS support. You can probably do this by using BScreen::DPMSCapabilities(). Then you need to disable the checkbox and slider for turning off the screen in case there is no support, but additionally, you need to add a BStringView below these controls explaining that DPMS is not available in the current graphics driver.

comment:8 Changed 10 years ago by vijay

Before submitting the patch.Check the attached screenshot guys.Does this look ok?

comment:9 Changed 10 years ago by stippi

You could remove the code that changes the slider bar width, so that the default width is used. Then you have a little more room perhaps. I would place the string view so that it aligns with the left side of the checkbox already. In Appearance, where there happens something similar with the subpixel anti-aliasing option, I have made the text use the italic font face. Those are the only (minor) suggestions I could give, otherwise it looks just fine! :-) Thanks for working on it!

comment:10 Changed 10 years ago by jackburton

I think that the sentence used is too technical. "DPMS" and "VGA Driver" shouldn't be used, IMHO. "Support unavailable", or "Power Management support not available" or something like this would sound better.

comment:11 in reply to:  10 Changed 10 years ago by vijay

Replying to jackburton:

I think that the sentence used is too technical. "DPMS" and "VGA Driver" shouldn't be used, IMHO. "Support unavailable", or "Power Management support not available" or something like this would sound better.

I agree, the sentence is a bit technical.I think i will go with "Power management support not available".

comment:12 Changed 10 years ago by koki

I think the placement of the message should be closer to where the user action takes place.

Ergo, IMO it is much more logical to place a "Power management not supported" text bit following (to the right) of the "Turn off screen" checkbox label (where "20 minutes" is), perhaps between parenthesis. And since the feature is not supported, the "20 minutes" text should not be displayed at all.

This would also allow you to even out the vertical space between sliders (it looks weird now).

FWIW and IMHO.

comment:13 in reply to:  9 Changed 10 years ago by vijay

Replying to stippi:

You could remove the code that changes the slider bar width, so that the default width is used. Then you have a little more room perhaps. I would place the string view so that it aligns with the left side of the checkbox already. In Appearance, where there happens something similar with the subpixel anti-aliasing option, I have made the text use the italic font face. Those are the only (minor) suggestions I could give, otherwise it looks just fine! :-) Thanks for working on it!

Thanks for the suggestions.Had a look at Appearence, greying out the string looks neat.I will do that here.

comment:14 in reply to:  12 Changed 10 years ago by vijay

Replying to koki:

I think the placement of the message should be closer to where the user action takes place.

Ergo, IMO it is much more logical to place a "Power management not supported" text bit following (to the right) of the "Turn off screen" checkbox label (where "20 minutes" is), perhaps between parenthesis. And since the feature is not supported, the "20 minutes" text should not be displayed at all.

This would also allow you to even out the vertical space between sliders (it looks weird now).

FWIW and IMHO.

I agree it will look definitely good if text is placed where "20 minutes" is.

comment:15 Changed 10 years ago by vijay

The below code snippet is taken from _UpdateTurnOffScreen() from ScreenSaverWindow.cpp

bool enabled = (fSettings.TimeFlags() & ENABLE_DPMS_MASK) != 0;

BScreen screen(this); uint32 dpmsCapabilities = screen.DPMSCapabilites(); fTurnOffScreenFlags = 0; if (dpmsCapabilities & B_DPMS_OFF)

fTurnOffScreenFlags |= ENABLE_DPMS_OFF;

if (dpmsCapabilities & B_DPMS_STAND_BY)

fTurnOffScreenFlags |= ENABLE_DPMS_STAND_BY;

if (dpmsCapabilities & B_DPMS_SUSPEND)

fTurnOffScreenFlags |= ENABLE_DPMS_SUSPEND;

fTurnOffCheckBox->SetValue(enabled && fTurnOffScreenFlags != 0

? B_CONTROL_ON : B_CONTROL_OFF);

i could not understand what fSettings.TimeFlags() returns and also i believe ENABLE_DPMS_MASK bits are not set in value returned by fSettings.TimeFlags().On what basis the ENABLE_DPMS_MASK bits will be set?

Changed 10 years ago by vijay

Attachment: screensaver.png added

comment:16 Changed 10 years ago by vijay

Check the latest screenshot.I could not get the text of TurnOff checkbox"Turn off screen" and the "Power management" text are not aligned(Horizontal) properly.italic font face

comment:17 Changed 10 years ago by vijay

Check the latest screenshot.I could not get the text of TurnOff checkbox"Turn off screen" and the "Power management" text are not aligned(Horizontal) properly.Is it possible to use italic font face in slider label??

comment:18 Changed 10 years ago by koki

On second thought, why display the "Turn off screen" checkbox and slider at all when power management is not supported? Why not just hide them in that case?

comment:19 in reply to:  18 Changed 10 years ago by vijay

Replying to koki:

On second thought, why display the "Turn off screen" checkbox and slider at all when power management is not supported? Why not just hide them in that case?

I would prefer to follow an approach which is used uniformly in apps across haiku for this kind of situations(where an apps feature depends on a driver).Putting the text below/above/removing controls all sounds good to me.But is there any standard way to do these things since this is the first time iam doing UI programming.If graying out unsupported features is used across apps then i believe it is more apt than removing controls.

comment:20 Changed 10 years ago by stippi

Well. The feature should still be visible. So greying out the "Turn off screen" check box, but removing the slider and replacing it with an explanation of why it's disabled could be an option. However, I am against simply hiding features completely, since that may just irritate users and may introduce problems with user guides and other information, which the seemingly does not apply anymore.

comment:21 Changed 8 years ago by phoudoin

Resolution: fixed
Status: reopenedclosed

Added in hrev40530 a text explaining why the Turn Off feature is not available, which hide the slider then.

Note: See TracTickets for help on using tickets.