PowerToys icon indicating copy to clipboard operation
PowerToys copied to clipboard

[Settings] Change icons and text

Open Jay-o-Way opened this issue 1 year ago • 26 comments

Summary of the Pull Request

  1. Updated some icons as from #10331 and (personal suggestions in) #18866.
    • General Page > "Run as admin" probably needs some tuning with the text and link. (did it in VS Code, please do check)
  2. Removed description which is incorrect for Win.11, as mentioned in #19590
  3. Tweak text for Camera overlay image #18573
  4. Added IsEnabled on Infobar on ScG page #16885
  5. Tweak colors to adress #16261 (might not be perfect yet, please do check)
  6. Use "primary/seconday", not "left/right" mouse buttons #19742
  • Feedback from pr

PR Checklist

  • [ ] Closes: #xxx
  • [x] Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
  • [ ] Tests: Added/updated and all pass
  • [ ] Localization: All end user facing strings can be localized
  • [ ] Dev docs: Added/updated
  • [ ] New binaries: Added on the required places
  • [ ] Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Jay-o-Way avatar Jul 23 '22 12:07 Jay-o-Way

@jay-o-way There is a warning box in PT Run's plugin manager that has an incorrect message. Can you update it?

The message is shown on forbidden keywords and not on multiple used keywords.

String id: Run_NotAllowedKeywordWarning.Title Current text is something like: This activation command is used for other plugins too. Better message: This activation command is forbidden or reserved for the use in other plugins.

htcfreek avatar Jul 24 '22 09:07 htcfreek

@htcfreek how do I trigger that message, again?

Using keywords like \ or \\

https://github.com/microsoft/PowerToys/blob/0acd8bacbbebca9cfd30f91f61ea7b4f862c84bb/src/settings-ui/Settings.UI.Library/ViewModels/PowerLauncherPluginViewModel.cs#L194-L202

https://github.com/microsoft/PowerToys/blob/0acd8bacbbebca9cfd30f91f61ea7b4f862c84bb/src/settings-ui/Settings.UI/Views/PowerLauncherPage.xaml#L310-L314

htcfreek avatar Jul 24 '22 10:07 htcfreek

And yes, I realize the "left/right" commits increase the number of files. Reason is that, when I thought of creating another branch just for that, the last commit wasn't a clean one: it contained changes in files unrelated to this subject. Trying to sort that out - without starting all over - was too difficult. Or I wasn't patient enough.

Jay-o-Way avatar Aug 03 '22 18:08 Jay-o-Way

@jay-o-way Are the left/rouse mouse button updates intentional? I understand updating labels, but why updating variable names in the core code (and were you able to test those changes)?

niels9001 avatar Aug 07 '22 06:08 niels9001

@jay-o-way Are the left/rouse mouse button updates intentional? I understand updating labels, but why updating variable names in the core code (and were you able to test those changes)?

Well, it started with those (two) user facing words, then the xaml bindings. After that I just wanted to keep going. Haven't been able to really test it. My computer is not that fast, and the bot doesn't give errors any more. That's why a check from you guys is always welcome.

Jay-o-Way avatar Aug 07 '22 08:08 Jay-o-Way

@jay-o-way Are the left/rouse mouse button updates intentional? I understand updating labels, but why updating variable names in the core code (and were you able to test those changes)?

Well, it started with those (two) user facing words, then the xaml bindings. After that I just wanted to keep going. Haven't been able to really test it. My computer is not that fast, and the bot doesn't give errors any more. That's why a check from you guys is always welcome.

Since this PR is about updating user-facing text labels and icons in Settings, I'd stick to that. Now you're touching multiple projects in a single PR (I'd not touch those var names anyways, since they are not user facing..).

If you revert those I'll do a pass on the icons and text labels and approve 😄

niels9001 avatar Aug 07 '22 08:08 niels9001

(Woops, my bad)

Jay-o-Way avatar Aug 08 '22 17:08 Jay-o-Way

I'm close to replacing my sluggish desktop with an all-in-one pc soon. Just waiting until I have both a "new" CPU and an SSD. And then I have to install literally everything - hardware and software. I'm hoping, by the time I'm done, I can do more with Visual Studio so I can actually test more myself. Until then: I still have a number of unanswered questions in this PR @niels9001 @stefansjfw @jaimecbernardo I hope this can be included in the next release 🤞🏻

Jay-o-Way avatar Aug 21 '22 18:08 Jay-o-Way

Hi @Jay-o-Way , Could you please point me at what's still unanswered to expedite? ;) Thank you!

jaimecbernardo avatar Aug 24 '22 11:08 jaimecbernardo

  1. For the Up and Down icons in the list: "Can I use <MenuFlyoutItem Icon="E74B"... /> here to remove <MenuFlyoutItem.Icon> ... </>, or will that fail too?"
  2. Apparently, the IsEnabled property doesn't seem to work on the <muxc:InfoBar x:Uid="ShortcutGuide_PressWinKeyWarning" on the VCM settings page. This might be a WinUI bug. Other muxc:infobars do change their visual style, but they are all constructed differently. Need a work-around for this one. To repeat myself for clarity: the IsOpen property has a different purpose and thus is not an alternative.

To Niels - not blocking:

  • That Windows11 converter/helper is a fine idea too - we already have one. Did you miss that?
  • I changed icon E790 "Color" to EB3C "Design" in a few instances. I don't see a "no-go", so I suppose that's good?
  • I changed the icon for ImageResizer_Presets → same question.
  • We've had a discusson about using check boxes in PowerRename UI earlier and I still stand behind that.

Jay-o-Way avatar Aug 24 '22 12:08 Jay-o-Way

  1. For the Up and Down icons in the list: "Can I use <MenuFlyoutItem Icon="E74B"... /> here to remove <MenuFlyoutItem.Icon> ... </>, or will that fail too?"
  2. Apparently, the IsEnabled property doesn't seem to work on the <muxc:InfoBar x:Uid="ShortcutGuide_PressWinKeyWarning" on the VCM settings page. This might be a WinUI bug. Other muxc:infobars do change their visual style, but they are all constructed differently. Need a work-around for this one. To repeat myself for clarity: the IsOpen property has a different purpose and thus is not an alternative.

To Niels - not blocking:

  • That Windows11 converter/helper is a fine idea too - we already have one. Did you miss that?
  • I changed icon E790 "Color" to EB3C "Design" in a few instances. I don't see a "no-go", so I suppose that's good?
  • I changed the icon for ImageResizer_Presets → same question.
  • We've had a discusson about using check boxes in PowerRename UI earlier and I still stand behind that.
  • Fine with the icon changes.
  • Just leave the MenuFlyoutItem code as is and change the glyph code for 'down'.
  • We can look at the InfoBar issues in a seperate PR - there's a tracking issue right?
  • CheckBoxes for PowerRename is not relevant to this PR? So would move that discussion to a dedicated issue. I do find the subfiles/folders to be a regression to the icon we have now. We can have a look at creating custom icons if necessary later?

niels9001 avatar Aug 24 '22 12:08 niels9001

Anything keeping this from being merged now?

Jay-o-Way avatar Aug 27 '22 11:08 Jay-o-Way

Anything keeping this from being merged now?

I can tsst it later this week and will merge if everything works as expected 👍👍

niels9001 avatar Aug 28 '22 11:08 niels9001

/azp run

crutkas avatar Sep 04 '22 23:09 crutkas

Azure Pipelines successfully started running 1 pipeline(s).

azure-pipelines[bot] avatar Sep 04 '22 23:09 azure-pipelines[bot]

Thank you! We've already got our Release Candidates, though, so it's going in for 0.63.

jaimecbernardo avatar Sep 05 '22 13:09 jaimecbernardo

@Jay-o-Way , @niels9001 , @jaimecbernardo Before merging this please validate that all icons are available on all supported os versions including Windows 10.

htcfreek avatar Sep 05 '22 20:09 htcfreek

@htcfreek all come from https://docs.microsoft.com/windows/apps/design/style/segoe-fluent-icons-font so it's good.

Jay-o-Way avatar Sep 12 '22 10:09 Jay-o-Way

in the future for stuff like changing icons, in the PR, would be super powerful to see before / after. Goes from quick glance to having to compile

crutkas avatar Sep 17 '22 04:09 crutkas

in the future for stuff like changing icons, in the PR, would be super powerful to see before / after. Goes from quick glance to having to compile

Can do. I had an rather old and very slow desktop. Now I have an all-in-one with SSD.

Jay-o-Way avatar Sep 17 '22 11:09 Jay-o-Way

@Jay-o-Way May I ask? I wanna know the question I raised, "Meaning-unknown Chinese characters in PowerToys UI", had been taken into consideration?

SteinwaySons avatar Sep 19 '22 01:09 SteinwaySons

@Jay-o-Way May I ask? I wanna know the question I raised, "Meaning-unknown Chinese characters in PowerToys UI", had been taken into consideration?

There is a deprecated icon code used for this icon. These still show in many (non-asian?) languages, therefore it wasn't obvious. It is changed in this pr.

Jay-o-Way avatar Sep 19 '22 10:09 Jay-o-Way

@Jay-o-Way May I ask? I wanna know the question I raised, "Meaning-unknown Chinese characters in PowerToys UI", had been taken into consideration?

There is a deprecated icon code used for this icon. These still show in many (non-asian?) languages, therefore it wasn't obvious. It is changed in this pr.

wht font do we use? static or symbolfont resource?

htcfreek avatar Sep 19 '22 10:09 htcfreek

@htcfreek it's not about the font but the (some deprecated) icon codes: https://learn.microsoft.com/windows/apps/design/style/segoe-fluent-icons-font#icon-list

Jay-o-Way avatar Sep 19 '22 11:09 Jay-o-Way

@htcfreek it's not about the font but the (some deprecated) icon codes: https://learn.microsoft.com/windows/apps/design/style/segoe-fluent-icons-font#icon-list

But why it's only on the Chinese system? Can you link me the code line please.

htcfreek avatar Sep 19 '22 14:09 htcfreek

But why it's only on the Chinese system? Can you link me the code line please.

No idea why. https://github.com/microsoft/PowerToys/blob/21393f0a9d99f56619febbcf84dfc348dc2e287a/src/settings-ui/Settings.UI/Controls/ShortcutControl/ShortcutControl.xaml#L43 (was E104)

Jay-o-Way avatar Sep 19 '22 14:09 Jay-o-Way

@Jay-o-Way May I ask? I wanna know the question I raised, "Meaning-unknown Chinese characters in PowerToys UI", had been taken into consideration?

@Jay-o-Way I just find out why my PT meets trouble in display icon. Because of font set missing! Someone gave us a solution on this link. Corresponding MS docs link is here. I copy the reply here.

Seems it's caused by the lack of segoe fluent icons, which is not included in Windows 10 by default. Installing the font solved this issue on my machine. Reference PS: this bug just appeared several versions ago, by when I've never installed this font

Here is the snapshot. image

So, after I download Segoe Fluent Icons font and install this font set, UI error is gone! image image image

It's work for me!

SteinwaySons avatar Sep 20 '22 09:09 SteinwaySons

@Jay-o-Way Finally, please accept my apology for misunderstanding. Hope PT will have a good future!

SteinwaySons avatar Sep 20 '22 10:09 SteinwaySons