-
Notifications
You must be signed in to change notification settings - Fork 6.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ColorPicker] Fix hex input without # in adjust color ui #14966
Conversation
@check-spelling-bot ReportUnrecognized words, please review:
Previously acknowledged words that are now absentAccessible Actioncenter Addavirtualdesktop ALIGNLEFT AUTOAPPEND AUTOSIZECOLUMNS available azurewebsites bca BEGINLABELEDIT bfee btn Captureascreenshot CASESENSITIVE checkboxes CHECKCANCELED chrisharris CIEXYZ coc COLUMNCLICK Connectquickaction coords countslabelrenamingfmt countslabelselectedfmt CRename CTriage dchristensen Deduplicate DISPINFO Displayandhidethedesktop djsoref docsmsft dogancelik DOUBLEBUFFER dupenv DWLP Easeof Entireitemname ENUMITEMS estdir EXCLUDEFILES EXCLUDEFOLDERS EXCLUDESUBFOLDERS EXISTINGIMAGERESIZERPATH EXISTINGPOWERRENAMEEXTPATH EXTENIONONLY EXTENSIONONLY FFAA fluentui Fody ftp ftps FULLNAME Gamebar GETDISPINFO GETEMPTYMARKUP gmx HACCEL HDF hdi HDITEM hdlg HDN HDS hitinfo htt ianjoneill IAuto IDrop INDEXTOSTATEIMAGEMASK inprivate installpowertoys IPreview IProgress ITEMSTATEICONCLICK IThumbnail itsme jakeoeding JSX KERNELBASE linecap Linkmenu listbox localhost Lockyour LPDWORD LPNMHDR LPNMHEADER LPNMLISTVIEW LPOLESTR LPPOINT lvc LVCF LVCFMT LVHITTESTINFO LVHT LVIF LVIS LVN LVS LVSIL MARQUEEPROGRESS MATCHMODE Mensching mfreadwrite mfuuid Minimizeallwindows MINMAXINFO NAMEONLY Nefario nitroin NMLVEMPTYMARKUP noactive Noactivewindow npm null nunit ONITEM Openthe OPTIONSGROUP pcelt PCorswitchaccounts pitem plvdi pnm pnmdr pnmlv POINTL polymorpism powertoyswiki ppenum ppsrui PREVIEWGROUP pritudev PROGDLG prpui Prt prui Radiobuttons RDW recyclebin refcount REPLACEWITH Reportx rgelt Rgn Rundialogbox Scn SEARCHFOR SEARCHREPLACEGROUP SHAREIMAGELISTS sidepanel SINGLESEL SORTDOWN spamming spdth sppd sppre spsrif spsrui STATEIMAGEMASK stringstream Switchbetweenvirtualdesktops systray Taskview Temporarilypeekatthedesktop textref THEMECHANGED TIMERID titlecase tsx uifabric uifabricicons ulazy UPDOWNKEYDROPSLIST USEREGEX windevbuildagents winstore XDiff xia XSmall XToolset xunit YDiff ZoomusingmagnifierSome files were were automatically ignoredThese sample patterns would exclude them:
You should consider adding them to:
File matching is via Perl regular expressions. To check these files, more of their words need to be in the dictionary than not. You can use To accept these unrecognized words as correct (and remove the previously acknowledged and now absent words), run the following commands... in a clone of the git@github.com:htcfreek/PowerToys.git repository
If you see a bunch of garbageIf it relates to a ... well-formed patternSee if there's a pattern that would match it. If not, try writing one and adding it to the Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines. Note that patterns can't match multiline strings. binary-ish stringPlease add a file path to the File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.
|
@check-spelling-bot ReportUnrecognized words, please review:
Previously acknowledged words that are now absentAccessible Actioncenter Addavirtualdesktop ALIGNLEFT AUTOAPPEND AUTOSIZECOLUMNS available azurewebsites bca BEGINLABELEDIT bfee btn Captureascreenshot CASESENSITIVE checkboxes CHECKCANCELED chrisharris CIEXYZ coc COLUMNCLICK Connectquickaction coords countslabelrenamingfmt countslabelselectedfmt CRename CTriage dchristensen Deduplicate DISPINFO Displayandhidethedesktop djsoref docsmsft dogancelik DOUBLEBUFFER dupenv DWLP Easeof Entireitemname ENUMITEMS estdir EXCLUDEFILES EXCLUDEFOLDERS EXCLUDESUBFOLDERS EXISTINGIMAGERESIZERPATH EXISTINGPOWERRENAMEEXTPATH EXTENIONONLY EXTENSIONONLY FFAA fluentui Fody ftp ftps FULLNAME Gamebar GETDISPINFO GETEMPTYMARKUP gmx HACCEL HDF hdi HDITEM hdlg HDN HDS hitinfo htt ianjoneill IAuto IDrop INDEXTOSTATEIMAGEMASK inprivate installpowertoys IPreview IProgress ITEMSTATEICONCLICK IThumbnail itsme jakeoeding JSX KERNELBASE linecap Linkmenu listbox localhost Lockyour LPDWORD LPNMHDR LPNMHEADER LPNMLISTVIEW LPOLESTR LPPOINT lvc LVCF LVCFMT LVHITTESTINFO LVHT LVIF LVIS LVN LVS LVSIL MARQUEEPROGRESS MATCHMODE Mensching mfreadwrite mfuuid Minimizeallwindows MINMAXINFO NAMEONLY Nefario nitroin NMLVEMPTYMARKUP noactive Noactivewindow npm null nunit ONITEM Openthe OPTIONSGROUP pcelt PCorswitchaccounts pitem plvdi pnm pnmdr pnmlv POINTL polymorpism powertoyswiki ppenum ppsrui PREVIEWGROUP pritudev PROGDLG prpui Prt prui Radiobuttons RDW recyclebin refcount REPLACEWITH Reportx rgelt Rgn Rundialogbox Scn SEARCHFOR SEARCHREPLACEGROUP SHAREIMAGELISTS sidepanel SINGLESEL SORTDOWN spamming spdth sppd sppre spsrif spsrui STATEIMAGEMASK stringstream Switchbetweenvirtualdesktops systray Taskview Temporarilypeekatthedesktop textref THEMECHANGED TIMERID titlecase tsx uifabric uifabricicons ulazy UPDOWNKEYDROPSLIST USEREGEX windevbuildagents winstore XDiff xia XSmall XToolset xunit YDiff ZoomusingmagnifierSome files were were automatically ignoredThese sample patterns would exclude them:
You should consider adding them to:
File matching is via Perl regular expressions. To check these files, more of their words need to be in the dictionary than not. You can use To accept these unrecognized words as correct (and remove the previously acknowledged and now absent words), run the following commands... in a clone of the git@github.com:htcfreek/PowerToys.git repository
If you see a bunch of garbageIf it relates to a ... well-formed patternSee if there's a pattern that would match it. If not, try writing one and adding it to the Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines. Note that patterns can't match multiline strings. binary-ish stringPlease add a file path to the File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.
|
@check-spelling-bot ReportUnrecognized words, please review:
Previously acknowledged words that are now absentAccessible Actioncenter Addavirtualdesktop ALIGNLEFT AUTOAPPEND AUTOSIZECOLUMNS available azurewebsites bca BEGINLABELEDIT bfee btn Captureascreenshot CASESENSITIVE checkboxes CHECKCANCELED chrisharris CIEXYZ coc COLUMNCLICK Connectquickaction coords countslabelrenamingfmt countslabelselectedfmt CRename CTriage dchristensen Deduplicate DISPINFO Displayandhidethedesktop djsoref docsmsft dogancelik DOUBLEBUFFER dupenv DWLP Easeof Entireitemname ENUMITEMS estdir EXCLUDEFILES EXCLUDEFOLDERS EXCLUDESUBFOLDERS EXISTINGIMAGERESIZERPATH EXISTINGPOWERRENAMEEXTPATH EXTENIONONLY EXTENSIONONLY FFAA fluentui Fody ftp ftps FULLNAME Gamebar GETDISPINFO GETEMPTYMARKUP gmx HACCEL HDF hdi HDITEM hdlg HDN HDS hitinfo htt ianjoneill IAuto IDrop INDEXTOSTATEIMAGEMASK inprivate installpowertoys IPreview IProgress ITEMSTATEICONCLICK IThumbnail itsme jakeoeding JSX KERNELBASE linecap Linkmenu listbox localhost Lockyour LPDWORD LPNMHDR LPNMHEADER LPNMLISTVIEW LPOLESTR LPPOINT lvc LVCF LVCFMT LVHITTESTINFO LVHT LVIF LVIS LVN LVS LVSIL MARQUEEPROGRESS MATCHMODE Mensching mfreadwrite mfuuid Minimizeallwindows MINMAXINFO NAMEONLY Nefario nitroin NMLVEMPTYMARKUP noactive Noactivewindow npm null nunit ONITEM Openthe OPTIONSGROUP pcelt PCorswitchaccounts pitem plvdi pnm pnmdr pnmlv POINTL polymorpism powertoyswiki ppenum ppsrui PREVIEWGROUP pritudev PROGDLG prpui Prt prui Radiobuttons RDW recyclebin refcount REPLACEWITH Reportx rgelt Rgn Rundialogbox Scn SEARCHFOR SEARCHREPLACEGROUP SHAREIMAGELISTS sidepanel SINGLESEL SORTDOWN spamming spdth sppd sppre spsrif spsrui STATEIMAGEMASK stringstream Switchbetweenvirtualdesktops systray Taskview Temporarilypeekatthedesktop textref THEMECHANGED TIMERID titlecase tsx uifabric uifabricicons ulazy UPDOWNKEYDROPSLIST USEREGEX windevbuildagents winstore XDiff xia XSmall XToolset xunit YDiff ZoomusingmagnifierSome files were were automatically ignoredThese sample patterns would exclude them:
You should consider adding them to:
File matching is via Perl regular expressions. To check these files, more of their words need to be in the dictionary than not. You can use To accept these unrecognized words as correct (and remove the previously acknowledged and now absent words), run the following commands... in a clone of the git@github.com:htcfreek/PowerToys.git repository
If you see a bunch of garbageIf it relates to a ... well-formed patternSee if there's a pattern that would match it. If not, try writing one and adding it to the Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines. Note that patterns can't match multiline strings. binary-ish stringPlease add a file path to the File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.
|
// Update hex text box to show code with # and fix cursor position | ||
if (insertHexTextHashtag) | ||
{ | ||
HexCode.Text = newValue; | ||
HexCode.Select(HexCode.Text.Length, 0); | ||
insertHexTextHashtag = false; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@niels9001, @stefansjfw
If you think we get problems with adding hashtag in real time while typing on slow systems, we can think about adding it only when the user switches focus to another control.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I think the better way to do it either on focus to another control , or on flyout closing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do it on focus change in the new pr.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stefansjfw
Do you mean we get in trouble whe the user switches focus to the slider and changes it immediately? (By clicking near the dot and moving it immediately.)
Maybe we should take care on slider value updates befor writing new text into the hex field. 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that will be a problem. But that's why I suggested to add # on flyout closing - it should be safer. We don't need to think about where would focus go and if it's going to affect the text field
var newValue = (sender as TextBox).Text; | ||
|
||
// support hex with 3 and 6 characters | ||
var reg = new Regex("^#([0-9A-Fa-f]{3}){1,2}$"); | ||
// support hex with 3 or 6 characters and with or without # |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stefansjfw , @niels9001
Here we accept three values input but we can't use it correctly. We interpret three values input as the last three values of a six values input.
Should we remove the matching of three digits input or should we add an converter?
I have opened an issue here: #14969
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I personally suggest to block three digits input untils the converter is implemted and the issue is fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not experienced with regex so @stefansfjw can better judge it.
FYI if Color Picker moves to WinUI3 we can remove all of this custom code and use the default WinUI component.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I closed this pr. I will create a new one in the next few weeks for fixing all bugs together. Otherwise we have to change code twice which is imo not necessary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Code layout update if I have to change code before merging again. They are optional.)
insertHexTextHashtag = false; | ||
} | ||
|
||
// Update other controls | ||
var converter = new System.Drawing.ColorConverter(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
var converter = new System.Drawing.ColorConverter(); | ||
|
||
var color = (System.Drawing.Color)converter.ConvertFromString(HexCode.Text); | ||
_ignoreHexChanges = true; | ||
var color = (System.Drawing.Color)converter.ConvertFromString(newValue); | ||
SetColorFromTextBoxes(color); | ||
_ignoreHexChanges = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_ignoreHexChanges = false; | |
_ignoreHexChanges = false; |
Closed PR to do all fixes (Hashtag, 3 values hex input, max length) in one step. |
Summary of the Pull Request
What is this about:
In the color edit dialog we currently only accept hex values with
#
. This is not user friendly as we removed the#
on the picked color code. When users copy the color code into the edit dialog hex field they have to add the#
manually.This PR updates UX to add the
#
automatically and to limit value length. The changes works with 3 and 6 value hex code.Note: The gif doesn't shows in real time (Don't know why.)
What is included in the PR:
#
.#
in text box if color code is valid.How does someone test / validate:
Build and tried manually with typing the code and inserting the code form clipboard with and without
#
.Quality Checklist
Contributor License Agreement (CLA)
A CLA must be signed. If not, go over here and sign the CLA.