Skip to content
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

Fixing the issue with HEX textbox not being updated with the correct RGB values #12936

Merged
merged 3 commits into from
Aug 30, 2021

Conversation

martinchrzan
Copy link
Contributor

@martinchrzan martinchrzan commented Aug 28, 2021

Summary of the Pull Request

What is this about:
When changing values in the RGB boxes, previous value was being used to calculate HEX color (and other representations), not the current one
Also fixing the issue when current color was changed only when pressed enter of textbox lost focus.

ModernWPF numberBox is not exposing events immediately when a value changes, I have added some code that goes into its internals and grab the actual value (with validating it properly)

What is include in the PR:
See above

How does someone test / validate:
Manual check
RGBTextBoxesFixes

Quality Checklist

Contributor License Agreement (CLA)

A CLA must be signed. If not, go over here and sign the CLA.

@martinchrzan martinchrzan changed the title Fixing the issue with hex textbox not being updated with the correct RGB values Fixing the issue with HEX textbox not being updated with the correct RGB values Aug 28, 2021
@niels9001
Copy link
Contributor

@martinchrzan will this by any chance also fix: #11108?

@martinchrzan
Copy link
Contributor Author

martinchrzan commented Aug 28, 2021

It will not, it still requires you to press enter or leave that textbox (numberbox) but I can have a look and add it here too.

@github-actions
Copy link

github-actions bot commented Aug 28, 2021

@check-spelling-bot Report

Unrecognized words, please review:

  • numberbox
Previously acknowledged words that are now absent Accessible coc djsoref dogancelik estdir Fody ftps gmx htt inprivate itsme mfreadwrite mfuuid Nefario nitroin null nunit Radiobuttons sidepanel ulazy XSmall xunit
Some files were were automatically ignored

These sample patterns would exclude them:

^src/modules/previewpane/UnitTests-MarkdownPreviewHandler/HelperFiles/MarkdownWithHTMLImageTag\.txt$

You should consider adding them to:

.github/actions/spell-check/excludes.txt

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 patterns.txt to exclude portions, add items to the dictionary (e.g. by adding them to allow.txt), or fix typos.

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:microsoft/PowerToys.git repository
on the user/martinchrzan/FixRGBToHEXBoxes branch:

update_files() {
perl -e '
my @expect_files=qw('".github/actions/spell-check/expect.txt"');
@ARGV=@expect_files;
my @stale=qw('"$patch_remove"');
my $re=join "|", @stale;
my $suffix=".".time();
my $previous="";
sub maybe_unlink { unlink($_[0]) if $_[0]; }
while (<>) {
if ($ARGV ne $old_argv) { maybe_unlink($previous); $previous="$ARGV$suffix"; rename($ARGV, $previous); open(ARGV_OUT, ">$ARGV"); select(ARGV_OUT); $old_argv = $ARGV; }
next if /^(?:$re)(?:(?:\r|\n)*$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spell-check/expect.txt";
use File::Path qw(make_path);
use File::Basename qw(dirname);
make_path (dirname($new_expect_file));
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"$patch_add"');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a)."-".$a cmp lc($b)."-".$b} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;
system("git", "add", $new_expect_file);
'
(cat '.github/actions/spell-check/excludes.txt' - <<EOF
$should_exclude_patterns
EOF
) |grep .|
sort -f |
uniq > '.github/actions/spell-check/excludes.txt.temp' &&
mv '.github/actions/spell-check/excludes.txt.temp' '.github/actions/spell-check/excludes.txt'
}

comment_json=$(mktemp)
curl -L -s -S \
  --header "Content-Type: application/json" \
  "https://api.github.com/repos/microsoft/PowerToys/issues/comments/907647941" > "$comment_json"
comment_body=$(mktemp)
jq -r .body < "$comment_json" > $comment_body
rm $comment_json

patch_remove=$(perl -ne 'next unless s{^</summary>(.*)</details>$}{$1}; print' < "$comment_body")
  

patch_add=$(perl -e '$/=undef;
$_=<>;
s{<details>.*}{}s;
s{^#.*}{};
s{\n##.*}{};
s{(?:^|\n)\s*\*}{}g;
s{\s+}{ }g;
print' < "$comment_body")
  

should_exclude_patterns=$(perl -e '$/=undef;
$_=<>;
exit unless s{(?:You should consider excluding directory paths|You should consider adding them to).*}{}s;
s{.*These sample patterns would exclude them:}{}s;
s{.*\`\`\`([^`]*)\`\`\`.*}{$1}m;
print' < "$comment_body" | grep . || true)

update_files
rm $comment_body
git add -u
If you see a bunch of garbage

If it relates to a ...

well-formed pattern

See if there's a pattern that would match it.

If not, try writing one and adding it to the patterns.txt file.

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 string

Please add a file path to the excludes.txt file instead of just accepting the garbage.

File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

@martinchrzan
Copy link
Contributor Author

@niels9001 Fixed that one as well now :)

/// <returns>Validated value as per numberbox conditions, if content is invalid it returns previous value</returns>
private static byte GetValueFromNumberBox(NumberBox numberBox)
{
var internalTextBox = GetChildOfType<TextBox>(numberBox);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested, worked as expected!

@niels9001
Copy link
Contributor

Any chance this pr could address #3420 too, or has that been already?

That's related to Run, right?

@Jay-o-Way
Copy link
Collaborator

Jay-o-Way commented Aug 29, 2021

Woops, i meant microsoft/microsoft-ui-xaml#3420 -> #7085 but that's for FancyZones color picking. Nevermind 🤐

@martinchrzan
Copy link
Contributor Author

It does not fix that, I am not suppressing any characters typed.

@martinchrzan martinchrzan merged commit 46bfd2c into master Aug 30, 2021
@martinchrzan martinchrzan deleted the user/martinchrzan/FixRGBToHEXBoxes branch August 30, 2021 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants