feat(color-picker-hex-input): auto apply new color after typing/pasting hex code #9561

aPreciado88 · 2024-06-11T20:18:07Z

Related Issue: #7057

Summary

Updates color-picker-hex-input to auto apply new color after typing or pasting valid hex value.

…ciado88/7057-apply-entered-hex-value-immediately

jcfranco

Looking good, @aPreciado88! 😎

jcfranco · 2024-06-14T17:10:21Z

packages/calcite-components/src/components/color-picker/color-picker.tsx

  handleKeyDown(event: KeyboardEvent): void {
    if (event.key === "Enter") {
      event.preventDefault();
    }
  }

+  handleInputFocus = (event: Event): void => {


Selecting text is a great enhancement, but it does seem unrelated to committing on paste/input. Could we submit this separately?

I'll create a separate issue for this.

jcfranco · 2024-06-14T17:10:51Z

packages/calcite-components/src/components/color-picker/color-picker.tsx

@@ -1034,6 +1034,8 @@ export class ColorPicker
        numberingSystem={this.numberingSystem}
        onCalciteInputNumberChange={this.handleChannelChange}
        onCalciteInputNumberInput={this.handleChannelInput}
+        onFocus={this.handleInputFocus}
+        onInput={this.handleChannelInputChange}


For consistency, can you use the calcite-specific events? Also, the event name doesn't match the event (e.g., named after the change event, but it's listening to input). Applies to the hex-input changes.

jcfranco · 2024-06-14T17:14:40Z

One more thing, committing value on blur or enter is by design, so this should use the feat type. This also matches the request.

…ciado88/7057-apply-entered-hex-value-immediately

jcfranco

Awesome! Two more things and this should be good to merge, can you:

update the test suite to cover this new workflow? I believe current tests use blur and Enter.
revisit the PR title to better match the feature type? fix color not auto updating is still bug-focused.

…ciado88/7057-apply-entered-hex-value-immediately

aPreciado88 · 2024-06-18T21:45:39Z

@jcfranco I updated the test suite and PR title. 🚀

jcfranco

🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨
🎨⌨️⌨️⌨️⌨️🎨⌨️🎨🎨🎨🎨🎨⌨️🎨⌨️⌨️⌨️⌨️🎨🎨⌨️⌨️⌨️🎨🎨⌨️⌨️🎨🎨⌨️🎨🎨🎨⌨️🎨⌨️⌨️⌨️⌨️🎨
🎨⌨️🎨🎨⌨️🎨⌨️🎨🎨🎨🎨🎨⌨️🎨⌨️🎨🎨🎨🎨⌨️🎨🎨🎨🎨⌨️🎨🎨⌨️🎨⌨️⌨️🎨⌨️⌨️🎨⌨️🎨🎨🎨🎨
🎨⌨️⌨️⌨️⌨️🎨⌨️🎨🎨⌨️🎨🎨⌨️🎨⌨️⌨️⌨️🎨🎨🎨⌨️⌨️🎨🎨⌨️🎨🎨⌨️🎨⌨️🎨⌨️🎨⌨️🎨⌨️⌨️⌨️🎨🎨
🎨⌨️🎨🎨⌨️🎨⌨️🎨⌨️🎨⌨️🎨⌨️🎨⌨️🎨🎨🎨🎨🎨🎨🎨⌨️🎨⌨️🎨🎨⌨️🎨⌨️🎨🎨🎨⌨️🎨⌨️🎨🎨🎨🎨
🎨⌨️🎨🎨⌨️🎨🎨⌨️🎨🎨🎨⌨️🎨🎨⌨️⌨️⌨️⌨️🎨⌨️⌨️⌨️🎨🎨🎨⌨️⌨️🎨🎨⌨️🎨🎨🎨⌨️🎨⌨️⌨️⌨️⌨️🎨
🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨🎨

Testing paste isn't possible due to Puppeteer limitations, right?

jcfranco · 2024-06-20T18:27:21Z

packages/calcite-components/src/components/color-picker-hex-input/color-picker-hex-input.e2e.ts

@@ -348,6 +348,14 @@ describe("calcite-color-picker-hex-input", () => {
          await assertTabAndEnterBehavior("", startingHex);
        });

+        it("commits hex chars when typing", async () => {
+          await selectText(input);
+          await page.keyboard.type("abcdef");


Since committing on paste/input only works for longhand hex, can you assert on "abc" not changing the value, then assert the value changed once "def" is typed?

I added assertion to only commit longhand hex values.

aPreciado88 · 2024-06-20T20:53:33Z

@jcfranco I tried to include a test for the copy-paste scenario. But it looks like the puppeteer limitations are still present.

jcfranco · 2024-06-20T20:54:06Z

packages/calcite-components/src/components/color-picker-hex-input/color-picker-hex-input.e2e.ts

          await page.waitForChanges();

+          // asserting that shorthand hex won't be committed
+          expect(await input.getProperty("value")).toBe("#b33f33");


You could use startingHex here instead of the actual hex value.

Also, I think the comments can be removed since the updated test name and assertions make it clear what the test entails.

I updated the test to use startingHex and removed the comments. 🚀

…ciado88/7057-apply-entered-hex-value-immediately

…diately

aPreciado88 requested a review from a team as a code owner June 11, 2024 20:18

aPreciado88 added this to the 2024-06-25 - Jun Release milestone Jun 11, 2024

aPreciado88 added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Jun 11, 2024

aPreciado88 changed the title ~~fix(color-picker-hex-input): fix color not updating after pasting or typing valid hex value~~ fix(color-picker-hex-input): fix color not auto updating Jun 11, 2024

fix(color-picker-hex-input): fix color not auto updating

6591424

aPreciado88 force-pushed the aPreciado88/7057-apply-entered-hex-value-immediately branch from a65fdb2 to 6591424 Compare June 11, 2024 20:25

aPreciado88 added skip visual snapshots Pull requests that do not need visual regression testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Jun 11, 2024

Merge branch 'dev' of github.com:Esri/calcite-design-system into aPre…

5043c9b

…ciado88/7057-apply-entered-hex-value-immediately

github-actions bot assigned aPreciado88 Jun 11, 2024

github-actions bot added the bug Bug reports for broken functionality. Issues should include a reproduction of the bug. label Jun 11, 2024

aPreciado88 added 4 commits June 12, 2024 08:30

fix(color-picker-hex-input): fix color not auto updating

54cd63e

fix(color-picker-hex-input): fix color not auto updating

69b82eb

fix(color-picker-hex-input): fix color not auto updating

3336d89

fix(color-picker-hex-input): fix color not auto updating

d53e0b2

jcfranco requested changes Jun 14, 2024

View reviewed changes

aPreciado88 changed the title ~~fix(color-picker-hex-input): fix color not auto updating~~ feat(color-picker-hex-input): fix color not auto updating Jun 14, 2024

aPreciado88 added enhancement Issues tied to a new feature or request. and removed bug Bug reports for broken functionality. Issues should include a reproduction of the bug. labels Jun 14, 2024

aPreciado88 added 3 commits June 17, 2024 14:27

fix(color-picker-hex-input): fix color not auto updating

2ceb323

fix(color-picker-hex-input): fix color not auto updating

1f2bb44

Merge branch 'dev' of github.com:Esri/calcite-design-system into aPre…

e305eeb

…ciado88/7057-apply-entered-hex-value-immediately

aPreciado88 added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed skip visual snapshots Pull requests that do not need visual regression testing. labels Jun 17, 2024

aPreciado88 requested a review from jcfranco June 17, 2024 22:34

aPreciado88 added skip visual snapshots Pull requests that do not need visual regression testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Jun 17, 2024

aPreciado88 added 2 commits June 17, 2024 15:43

feat(color-picker-hex-input): fix color not auto updating

b582603

feat(color-picker-hex-input): fix color not auto updating

1f47df2

Merge branch 'dev' of github.com:Esri/calcite-design-system into aPre…

5d94a47

…ciado88/7057-apply-entered-hex-value-immediately

jcfranco requested changes Jun 18, 2024

View reviewed changes

aPreciado88 changed the title ~~feat(color-picker-hex-input): fix color not auto updating~~ feat(color-picker-hex-input): auto apply new color after typing/pasting hex code Jun 18, 2024

aPreciado88 added 2 commits June 18, 2024 08:43

Merge branch 'dev' of github.com:Esri/calcite-design-system into aPre…

551af62

…ciado88/7057-apply-entered-hex-value-immediately

fix(color-picker-hex-input): update unit tests

13a099a

aPreciado88 requested a review from jcfranco June 18, 2024 21:45

jcfranco approved these changes Jun 20, 2024

View reviewed changes

fix(color-picker-hex-input): update unit tests

d5977f5

jcfranco reviewed Jun 20, 2024

View reviewed changes

aPreciado88 and others added 3 commits June 20, 2024 14:06

fix(color-picker-hex-input): update unit tests

98eacc9

Merge branch 'dev' of github.com:Esri/calcite-design-system into aPre…

af12d82

…ciado88/7057-apply-entered-hex-value-immediately

Merge branch 'dev' into aPreciado88/7057-apply-entered-hex-value-imme…

f151102

…diately

aPreciado88 merged commit 8b34583 into dev Jun 20, 2024
9 checks passed

aPreciado88 deleted the aPreciado88/7057-apply-entered-hex-value-immediately branch June 20, 2024 23:17

benelan mentioned this pull request Aug 28, 2024

chore: release main #10171

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(color-picker-hex-input): auto apply new color after typing/pasting hex code #9561

feat(color-picker-hex-input): auto apply new color after typing/pasting hex code #9561

aPreciado88 commented Jun 11, 2024 •

edited

Loading

jcfranco left a comment

jcfranco Jun 14, 2024

aPreciado88 Jun 17, 2024

jcfranco Jun 14, 2024

jcfranco commented Jun 14, 2024

jcfranco left a comment

aPreciado88 commented Jun 18, 2024

jcfranco left a comment

jcfranco Jun 20, 2024

aPreciado88 Jun 20, 2024

aPreciado88 commented Jun 20, 2024

jcfranco Jun 20, 2024

aPreciado88 Jun 20, 2024

feat(color-picker-hex-input): auto apply new color after typing/pasting hex code #9561

feat(color-picker-hex-input): auto apply new color after typing/pasting hex code #9561

Conversation

aPreciado88 commented Jun 11, 2024 • edited Loading

Summary

jcfranco left a comment

Choose a reason for hiding this comment

jcfranco Jun 14, 2024

Choose a reason for hiding this comment

aPreciado88 Jun 17, 2024

Choose a reason for hiding this comment

jcfranco Jun 14, 2024

Choose a reason for hiding this comment

jcfranco commented Jun 14, 2024

jcfranco left a comment

Choose a reason for hiding this comment

aPreciado88 commented Jun 18, 2024

jcfranco left a comment

Choose a reason for hiding this comment

jcfranco Jun 20, 2024

Choose a reason for hiding this comment

aPreciado88 Jun 20, 2024

Choose a reason for hiding this comment

aPreciado88 commented Jun 20, 2024

jcfranco Jun 20, 2024

Choose a reason for hiding this comment

aPreciado88 Jun 20, 2024

Choose a reason for hiding this comment

aPreciado88 commented Jun 11, 2024 •

edited

Loading