[FindMyMouse]Add additional settings #14590

jaimecbernardo · 2021-11-23T00:11:55Z

Summary of the Pull Request

What is this about:
Add more settings to Find My Mouse.

What is include in the PR:
Settings for:

Background color
Spotlight color
Overlay opacity
Spotlight radius
How bigger the spotlight is when the animation starts
Animation duration (0 is instant)

How does someone test / validate:
Change some settings and see them applied to Find My Mouse.

Quality Checklist

htcfreek · 2021-11-23T01:10:16Z

@jaimecbernardo
Great. Love this options. What about the "don't activate on fullscreen windows setting"?

htcfreek · 2021-11-23T01:17:40Z

@jaimecbernardo
Some small nits:

Can we add the unit to the description of Overlay opacity and imitial zoom? Would help users to understand the setting.
We use an other naming layout. Only the first letter should be big. (Example: Spotlight Color must be Spotlight color)

jaimecbernardo · 2021-11-23T01:22:18Z

@jaimecbernardo Great. Love this options. What about the "don't activate on fullscreen windows setting"?

Focusing on appearance for this PR.
But thanks for reminding :)

jaimecbernardo · 2021-11-23T01:23:47Z

Can we add the unit to the description of Overlay opacity and imitial zoom? Would help users to understand the setting.

@htcfreek What do you mean with unit?

htcfreek · 2021-11-23T06:13:03Z

Can we add the unit to the description of Overlay opacity and imitial zoom? Would help users to understand the setting.

@htcfreek What do you mean with unit?

What do I set here? Opacity in percent from 0 to 100? Zoom in pixel from x to y or in percentage from 0 to 100?

Maybe a number box is better for the zoom setting?

Jay-o-Way · 2021-11-23T08:14:31Z

Text like "How bigger the spotlight is" doesn't sound very correct.
we could group 'background' and 'opacity' together, then all the spotlight settings are together too.
Opacity: 0-255 or 0-100%? (Percentage feels more natural; zero percent makes it kind of useless; to me)
no max for delay?

Should we mention the bounds for values in the description or not?

niels9001 · 2021-11-23T08:41:35Z

@jaimecbernardo Just a general thought: I wonder if we should auto collapse the appearance and behavior settings for Mouse utilities by default? The extensive customization options are great, but might 'overwhelm' users when opening the page. Especially since we have multiple utilities on a single page.

jaimecbernardo · 2021-11-23T09:02:39Z

@htcfreek

What do I set here? Opacity in percent from 0 to 100? Zoom in pixel from x to y or in percentage from 0 to 100?

You're right, it's an alpha value. Perhaps I'll make it 100.
Regarding the zoom, it's how many times, meaning 1x, 2x, 3x, 4x (default is 9x). I made it configurable mostly for photo sensitive users for who that initial flash when the spotlight is big causes some pain. The idea here is that it doesn't need to be too configurable.

Maybe a number box is better for the zoom setting?

Doesn't need to be too configurable, although perhaps I'll raise the maximum a bit for this issue: #14539

stefansjfw · 2021-11-23T09:16:06Z

Background color colorpicker doesn't work for me. Whatever I select it stays black

jaimecbernardo · 2021-11-23T09:21:03Z

Background color colorpicker doesn't work for me. Whatever I select it stays black

@stefansjfw ,
You have to up brightness (black is described as 0 brightness in this control, which affects all other colors):

stefansjfw · 2021-11-23T09:22:51Z

Background color colorpicker doesn't work for me. Whatever I select it stays black

@stefansjfw , You have to up brightness (black is described as 0 brightness in this control, which affects all other colors):

Ok. It works :)

jaimecbernardo · 2021-11-23T09:27:55Z

@Jay-o-Way

Text like "How bigger the spotlight is" doesn't sound very correct.

Changed to "how many times larger".
Regarding the zoom, it's how many times, meaning 1x, 2x, 3x, 4x (default is 9x). I made it configurable mostly for photo sensitive users for who that initial flash when the spotlight is big causes some pain. The idea here is that it doesn't need to be too configurable.

we could group 'background' and 'opacity' together, then all the spotlight settings are together too.

Placed opacity at the beginning.

Opacity: 0-255 or 0-100%? (Percentage feels more natural; zero percent makes it kind of useless; to me)

Changed to percent. Made the minimum 1%.

no max for delay?

What would the maximum delay here be? I agree it is somewhat non-sensical to have very large numbers, but I also saw no reason to limit this.

Should we mention the bounds for values in the description or not?

The reason, in my opinion, for not adding bounds to the description is that then those would have to go through the localization cycle if we changed those, which means we could have some outdated

jaimecbernardo · 2021-11-23T09:31:43Z

@jaimecbernardo Just a general thought: I wonder if we should auto collapse the appearance and behavior settings for Mouse utilities by default? The extensive customization options are great, but might 'overwhelm' users when opening the page. Especially since we have multiple utilities on a single page.

Sounds like a good change. Applied it.

jaimecbernardo · 2021-11-23T09:33:48Z

Thanks for the feedback. I've committed change or answered to the suggestions so far. Updating description screenshot as well. PTAL

src/settings-ui/Microsoft.PowerToys.Settings.UI/Strings/en-us/Resources.resw

htcfreek · 2021-11-23T09:52:18Z

As I understood the zoom factor setting is important for photosensitive users. Should we add a warning bar under the setting, that they should not use a too small value?

stefansjfw

Implementation looks good. Works good as well

jaimecbernardo · 2021-11-23T10:55:39Z

As I understood the zoom factor setting is important for photosensitive users. Should we add a warning bar under the setting, that they should not use a too small value?

In this case the issue is with big values. A warning makes sense. Added the change and updating the image in the description as well.

stefansjfw · 2021-11-23T11:50:32Z

As I understood the zoom factor setting is important for photosensitive users. Should we add a warning bar under the setting, that they should not use a too small value?

In this case the issue is with big values. A warning makes sense. Added the change and updating the image in the description as well.

Nice!

htcfreek · 2021-11-23T12:26:26Z

As I understood the zoom factor setting is important for photosensitive users. Should we add a warning bar under the setting, that they should not use a too small value?

In this case the issue is with big values. A warning makes sense. Added the change and updating the image in the description as well.

Nice!

Does it make sense to bind the visibility on the slider value. For example when it's higher than 50% show the warning? 🤔

niels9001 · 2021-11-23T12:44:10Z

Why not add the warning in the Description instead of an InfoBar (for the sake of leaving out as much as UI as possible, since the list is growing :))? This will also not flag us for Accesibility related things as the InfoBar should be announced when selecting the previous setting.

Spotlight initial zoom
A smaller size is recommended for photo sensitive users

niels9001 · 2021-11-23T12:45:59Z

src/settings-ui/Microsoft.PowerToys.Settings.UI/Strings/en-us/Resources.resw

+    <value>Overlay opacity</value>
+  </data>
+  <data name="MouseUtils_FindMyMouse_OverlayOpacity.Description" xml:space="preserve">
+    <value>Opacity percentage for the overlay</value>


Isn't the title already self-explainatory? Doesn't add a lot and basically says the same thing twice.

Both @Jay-o-Way and @htcfreek asked for it:

[FindMyMouse]Add additional settings #14590 (comment)

[FindMyMouse]Add additional settings #14590 (comment)

@niels9001 , so perhaps if I just add make the description "Percentage"? Does this make sense?

What about simply adding the unit to the end of the first text?
Spotlight radius (px/pixels)
Spotlight inital zoom (factor)
Overlay opacity (%/percentage)
Animation duration (ms/milliseconds)

Then (imo) only the zoom factor would need a secondary text.

#KISS (keep it short and simple)

I think this should be resolved in a way that it's flexible and can be used across all modules (e.g. setting the opacity for Shortcut Guide overlay, or Zone Opacity for Fancy Zones).

One elegant solution could be adding it to the tooltip:

Other option is to add it after the input control. E.g.:
[ NumberBox ] ms
[ Slider ] %

I've created #14607 for this so we can keep this as is and @jaimecbernardo can continue doing awesome things :)!

Thank you 😄
So I've removed the description for now.
PTAL and approve if you think it's ready 😉

jaimecbernardo · 2021-11-23T13:26:02Z

Why not add the warning in the Description instead of an InfoBar (for the sake of leaving out as much as UI as possible, since the list is growing :))? This will also not flag us for Accesibility related things as the InfoBar should be announced when selecting the previous setting.

Spotlight initial zoom A smaller size is recommended for photo sensitive users

@niels9001 , like this?

src/settings-ui/Microsoft.PowerToys.Settings.UI/Strings/en-us/Resources.resw

crutkas · 2021-11-23T15:35:40Z

I don’t want to make that claim without having the low vision board for photo sensitive. Let’s please remove that statement and have them test it. There are tons of nuances here so I don’t want to make assumptions

jaimecbernardo · 2021-11-23T16:15:09Z

I don’t want to make that claim without having the low vision board for photo sensitive. Let’s please remove that statement and have them test it. There are tons of nuances here so I don’t want to make assumptions

@crutkas ,
I've committed the change and updated the screenshot in the PR's description.

jaimecbernardo · 2021-11-23T16:18:59Z

@niels9001 , Waiting for your approval and I think this can go in after that.
Trying to get this out for 0.51 RC.
Thank you, in advance.

niels9001

LGTM!

jaimecbernardo added 2 commits November 22, 2021 23:33

[FindMyMouse]Add additional settings

77f9f7c

Add setting for Spotlight Initial Zoom

a2115d9

jaimecbernardo requested review from niels9001, stefansjfw and crutkas November 23, 2021 00:11

PR feedback: lowercase settings names

dc5d3f8

PR feedback: settings descriptions

189523c

PR feedback: change opacity to percentage

b6d86c7

PR feedback: increase maximum zoom factor

3d20562

PR feedback: group spotlight settings

0c3f97c

PR feedback: Expanders start collapsed initially

765692c

htcfreek reviewed Nov 23, 2021

View reviewed changes

src/settings-ui/Microsoft.PowerToys.Settings.UI/Strings/en-us/Resources.resw Outdated Show resolved Hide resolved

stefansjfw approved these changes Nov 23, 2021

View reviewed changes

jaimecbernardo added 2 commits November 23, 2021 10:21

Merge branch 'main' into findmymouse-more-settings

197b10f

Remove extra settings file save in dllmain

2169b3f

jaimecbernardo added 2 commits November 23, 2021 10:31

PR feedback: change initial zoom description

9473a7f

PR feedback: Add warning for photo sensitive users

baa2f1a

niels9001 requested changes Nov 23, 2021

View reviewed changes

PR feedback: remove warning and add description instead

a008eed

htcfreek suggested changes Nov 23, 2021

View reviewed changes

src/settings-ui/Microsoft.PowerToys.Settings.UI/Strings/en-us/Resources.resw Outdated Show resolved Hide resolved

PR feedback: size->factor in initial zoom description

ee1ccec

niels9001 mentioned this pull request Nov 23, 2021

[Settings] Add units to settings #14607

Closed

Feedback PR: remove opacity description

ad3f1d9

PR feedback: remove photo sensitivity warning

515b48d

niels9001 self-requested a review November 23, 2021 16:33

niels9001 approved these changes Nov 23, 2021

View reviewed changes

jaimecbernardo merged commit 46244e8 into microsoft:main Nov 23, 2021

Jay-o-Way added the Product-Mouse Utilities Refers to the Mouse Utilities PowerToy label Nov 24, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FindMyMouse]Add additional settings #14590

[FindMyMouse]Add additional settings #14590

jaimecbernardo commented Nov 23, 2021 •

edited

htcfreek commented Nov 23, 2021

htcfreek commented Nov 23, 2021

jaimecbernardo commented Nov 23, 2021

jaimecbernardo commented Nov 23, 2021

htcfreek commented Nov 23, 2021 •

edited

Jay-o-Way commented Nov 23, 2021 •

edited

niels9001 commented Nov 23, 2021

jaimecbernardo commented Nov 23, 2021

stefansjfw commented Nov 23, 2021

jaimecbernardo commented Nov 23, 2021

stefansjfw commented Nov 23, 2021

jaimecbernardo commented Nov 23, 2021

jaimecbernardo commented Nov 23, 2021

jaimecbernardo commented Nov 23, 2021

htcfreek commented Nov 23, 2021

stefansjfw left a comment •

edited

jaimecbernardo commented Nov 23, 2021

stefansjfw commented Nov 23, 2021

htcfreek commented Nov 23, 2021

niels9001 commented Nov 23, 2021 •

edited

niels9001 Nov 23, 2021

jaimecbernardo Nov 23, 2021

jaimecbernardo Nov 23, 2021

Jay-o-Way Nov 23, 2021 •

edited

niels9001 Nov 23, 2021 •

edited

jaimecbernardo Nov 23, 2021

jaimecbernardo commented Nov 23, 2021

crutkas commented Nov 23, 2021

jaimecbernardo commented Nov 23, 2021

jaimecbernardo commented Nov 23, 2021

niels9001 left a comment

[FindMyMouse]Add additional settings #14590

[FindMyMouse]Add additional settings #14590

Conversation

jaimecbernardo commented Nov 23, 2021 • edited

Summary of the Pull Request

Quality Checklist

htcfreek commented Nov 23, 2021

htcfreek commented Nov 23, 2021

jaimecbernardo commented Nov 23, 2021

jaimecbernardo commented Nov 23, 2021

htcfreek commented Nov 23, 2021 • edited

Jay-o-Way commented Nov 23, 2021 • edited

niels9001 commented Nov 23, 2021

jaimecbernardo commented Nov 23, 2021

stefansjfw commented Nov 23, 2021

jaimecbernardo commented Nov 23, 2021

stefansjfw commented Nov 23, 2021

jaimecbernardo commented Nov 23, 2021

jaimecbernardo commented Nov 23, 2021

jaimecbernardo commented Nov 23, 2021

htcfreek commented Nov 23, 2021

stefansjfw left a comment • edited

Choose a reason for hiding this comment

jaimecbernardo commented Nov 23, 2021

stefansjfw commented Nov 23, 2021

htcfreek commented Nov 23, 2021

niels9001 commented Nov 23, 2021 • edited

niels9001 Nov 23, 2021

Choose a reason for hiding this comment

jaimecbernardo Nov 23, 2021

Choose a reason for hiding this comment

jaimecbernardo Nov 23, 2021

Choose a reason for hiding this comment

Jay-o-Way Nov 23, 2021 • edited

Choose a reason for hiding this comment

niels9001 Nov 23, 2021 • edited

Choose a reason for hiding this comment

jaimecbernardo Nov 23, 2021

Choose a reason for hiding this comment

jaimecbernardo commented Nov 23, 2021

crutkas commented Nov 23, 2021

jaimecbernardo commented Nov 23, 2021

jaimecbernardo commented Nov 23, 2021

niels9001 left a comment

Choose a reason for hiding this comment

jaimecbernardo commented Nov 23, 2021 •

edited

htcfreek commented Nov 23, 2021 •

edited

Jay-o-Way commented Nov 23, 2021 •

edited

stefansjfw left a comment •

edited

niels9001 commented Nov 23, 2021 •

edited

Jay-o-Way Nov 23, 2021 •

edited

niels9001 Nov 23, 2021 •

edited