fix(circuit-ui): ColorInput paste and change events #2687

matoous · 2024-09-17T11:44:58Z

Improve ColorInput, specifically:

handle default value and value props properly
ensure color picker and hex color input are always synchronized
handle properly paste events with hex color

changeset-bot · 2024-09-17T11:45:02Z

🦋 Changeset detected

Latest commit: db0b401

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@sumup/circuit-ui	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

vercel · 2024-09-17T11:45:02Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
oss-circuit-ui	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 12, 2024 11:54am

codecov · 2024-09-17T11:49:41Z

Codecov Report

Attention: Patch coverage is 96.15385% with 2 lines in your changes missing coverage. Please review.

Project coverage is 86.74%. Comparing base (14643b2) to head (db0b401).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...es/circuit-ui/components/ColorInput/ColorInput.tsx	96.15%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2687      +/-   ##
==========================================
+ Coverage   86.63%   86.74%   +0.11%     
==========================================
  Files         210      210              
  Lines       12000    12043      +43     
  Branches     1507     1520      +13     
==========================================
+ Hits        10396    10447      +51     
+ Misses       1551     1543       -8     
  Partials       53       53

Files with missing lines	Coverage Δ
...es/circuit-ui/components/ColorInput/ColorInput.tsx	`98.10% <96.15%> (+7.66%)`	⬆️

.changeset/beige-planes-shave.md

packages/circuit-ui/components/ColorInput/ColorInput.spec.tsx

connor-baer · 2024-10-12T08:24:53Z

packages/circuit-ui/components/ColorInput/ColorInput.spec.tsx

+      expect(colorPicker.value).toBe('#000000');
+      expect(colorInput.value).toBe('');


Why are the values different here?

#000000 is the default for color picker, to my understanding it can never not have a value whereas the color input can be empty - meaning no color submitted.

packages/circuit-ui/components/ColorInput/ColorInput.tsx

connor-baer · 2024-10-12T08:30:17Z

packages/circuit-ui/components/ColorInput/ColorInput.tsx

+
+      const pastedText = e.clipboardData.getData('text/plain').trim();
+
+      if (!pastedText || !/^#[0-9A-F]{6}$/i.test(pastedText)) {


Suggested change

if (!pastedText || !/^#[0-9A-F]{6}$/i.test(pastedText)) {

if (!pastedText || !/^#?[0-9A-F]{3,6}$/i.test(pastedText)) {

I think the # should be optional, but then you also need to add it when setting the value of the color picker input.

Adjusted for pasting without #. Allowing 3-char colors is a little bit more tricky because html color input only support 7-character strings (6 for color + #)¹.

Footnotes

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/color ↩

connor-baer · 2024-10-12T08:33:36Z

packages/circuit-ui/components/ColorInput/ColorInput.tsx

    const onPickerColorChange: ChangeEventHandler<InputElement> = (e) => {
-      setCurrentColor(e.target.value);
+      if (colorInputRef.current) {
+        colorInputRef.current.value = e.target.value.replace('#', '');


Might be worth to extract the formatting logic for the color input and color picker values into reusable functions.

Tried and honestly didn't find it more readable. I think with just # even these inline calls should be understandable as long as all of them happen inside ColorInput. Happy to adjust though if you have specific suggestion how.

packages/circuit-ui/components/ColorInput/ColorInput.tsx

Improve ColorInput, specifically: - handle default value and value props properly - ensure color picker and hex color input are always synchronized - handle properly paste events with hex color

connor-baer

Nice, thank you! 🙌

matoous · 2024-10-12T12:13:15Z

@connor-baer can I merge? Or anything left? 🙈

connor-baer · 2024-10-12T12:28:37Z

@matoous Feel free to merge

matoous self-assigned this Sep 17, 2024

github-actions bot added the 🗂 circuit-ui label Sep 17, 2024

vercel bot deployed to Preview September 17, 2024 11:46 View deployment

matoous force-pushed the md/color-input-fixups branch from 08457d2 to cbe2b7f Compare September 17, 2024 12:55

vercel bot deployed to Preview September 17, 2024 12:57 View deployment

matoous force-pushed the md/color-input-fixups branch from cbe2b7f to 16ceaee Compare September 17, 2024 13:00

vercel bot deployed to Preview September 17, 2024 13:02 View deployment

vercel bot deployed to Preview September 17, 2024 13:18 View deployment

connor-baer reviewed Oct 12, 2024

View reviewed changes

matoous added 3 commits October 12, 2024 13:24

fix(circuit-ui): ColorInput paste and change events

424d09f

Improve ColorInput, specifically: - handle default value and value props properly - ensure color picker and hex color input are always synchronized - handle properly paste events with hex color

feat: changeset

165cf94

fix: changelog

5c66fc4

matoous force-pushed the md/color-input-fixups branch from d8364e0 to 2a58c47 Compare October 12, 2024 11:35

matoous requested a review from a team as a code owner October 12, 2024 11:35

matoous requested a review from sirineJ October 12, 2024 11:35

matoous force-pushed the md/color-input-fixups branch from 2a58c47 to 2f4973e Compare October 12, 2024 11:36

vercel bot deployed to Preview October 12, 2024 11:39 View deployment

address code review

5ecbb4f

matoous force-pushed the md/color-input-fixups branch from 2f4973e to 5ecbb4f Compare October 12, 2024 11:40

vercel bot deployed to Preview October 12, 2024 11:41 View deployment

matoous force-pushed the md/color-input-fixups branch from 121a167 to c678dc4 Compare October 12, 2024 11:51

fix: bubble events

db0b401

matoous force-pushed the md/color-input-fixups branch from c678dc4 to db0b401 Compare October 12, 2024 11:52

matoous requested a review from connor-baer October 12, 2024 11:54

vercel bot deployed to Preview October 12, 2024 11:54 View deployment

connor-baer approved these changes Oct 12, 2024

View reviewed changes

matoous merged commit 2e85454 into main Oct 12, 2024
13 of 14 checks passed

matoous deleted the md/color-input-fixups branch October 12, 2024 12:41

connor-baer mentioned this pull request Oct 12, 2024

Version Packages #2716

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(circuit-ui): ColorInput paste and change events #2687

fix(circuit-ui): ColorInput paste and change events #2687

matoous commented Sep 17, 2024

changeset-bot bot commented Sep 17, 2024 •

edited

Loading

vercel bot commented Sep 17, 2024 •

edited

Loading

codecov bot commented Sep 17, 2024 •

edited

Loading

connor-baer Oct 12, 2024

matoous Oct 12, 2024

connor-baer Oct 12, 2024

connor-baer Oct 12, 2024

matoous Oct 12, 2024

connor-baer Oct 12, 2024

matoous Oct 12, 2024

connor-baer left a comment

matoous commented Oct 12, 2024

connor-baer commented Oct 12, 2024

		expect(colorPicker.value).toBe('#000000');
		expect(colorInput.value).toBe('');


		const pastedText = e.clipboardData.getData('text/plain').trim();

		if (!pastedText \|\| !/^#[0-9A-F]{6}$/i.test(pastedText)) {

	if (!pastedText \|\| !/^#[0-9A-F]{6}$/i.test(pastedText)) {
	if (!pastedText \|\| !/^#?[0-9A-F]{3,6}$/i.test(pastedText)) {

fix(circuit-ui): ColorInput paste and change events #2687

fix(circuit-ui): ColorInput paste and change events #2687

Conversation

matoous commented Sep 17, 2024

changeset-bot bot commented Sep 17, 2024 • edited Loading

🦋 Changeset detected

vercel bot commented Sep 17, 2024 • edited Loading

codecov bot commented Sep 17, 2024 • edited Loading

Codecov Report

connor-baer Oct 12, 2024

Choose a reason for hiding this comment

matoous Oct 12, 2024

Choose a reason for hiding this comment

connor-baer Oct 12, 2024

Choose a reason for hiding this comment

connor-baer Oct 12, 2024

Choose a reason for hiding this comment

matoous Oct 12, 2024

Choose a reason for hiding this comment

Footnotes

connor-baer Oct 12, 2024

Choose a reason for hiding this comment

matoous Oct 12, 2024

Choose a reason for hiding this comment

connor-baer left a comment

Choose a reason for hiding this comment

matoous commented Oct 12, 2024

connor-baer commented Oct 12, 2024

changeset-bot bot commented Sep 17, 2024 •

edited

Loading

vercel bot commented Sep 17, 2024 •

edited

Loading

codecov bot commented Sep 17, 2024 •

edited

Loading