feat(chart) : Add Scatter and Bubble Chart Components #9151
+1,316
−46
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,11 +26,76 @@ export function ChartCopyButton({ | |
| const [hasCopied, setHasCopied] = React.useState(false) | ||
|
|
||
| React.useEffect(() => { | ||
| if (hasCopied) { | ||
| const timer = setTimeout(() => { | ||
| setHasCopied(false) | ||
| }, 2000) | ||
| return () => clearTimeout(timer) | ||
| } | ||
| }, [hasCopied]) | ||
|
|
||
| /** | ||
| * Copies text to clipboard using the modern Clipboard API with fallback support. | ||
|
|
||
| * Modern approach (preferred): | ||
| * - Uses `navigator.clipboard.writeText()` which is async and secure | ||
| * - Only available in secure contexts (HTTPS, localhost, or file://) | ||
| * - Supported in all modern browsers (Chrome 63+, Firefox 53+, Safari 13.1+, Edge 79+) | ||
|
|
||
| * Fallback approach (legacy): | ||
| * - Uses `document.execCommand('copy')` for older browsers | ||
| * - Works in non-secure contexts (HTTP) | ||
| * - Note: This method is deprecated and may be removed in future browser versions | ||
| * - Used as a best-effort compatibility layer for legacy environments | ||
| * - Required for: IE 11, older Android browsers, and HTTP-served pages | ||
| */ | ||
| const copyToClipboard = React.useCallback( | ||
| async (text: string) => { | ||
| try { | ||
| // Prefer the modern async clipboard API when available in secure contexts (HTTPS) | ||
| if (navigator.clipboard && window.isSecureContext) { | ||
| await navigator.clipboard.writeText(text) | ||
| } else { | ||
| // Fallback for older browsers or non-secure contexts (e.g., HTTP, some iframes) | ||
| // This path handles: | ||
| // 1. Older browsers that don't support navigator. clipboard | ||
| // 2. Non-secure contexts (HTTP instead of HTTPS) | ||
| // 3. Browsers with clipboard API disabled | ||
| const textArea = document.createElement("textarea") | ||
| textArea.value = text | ||
| // Position off-screen to avoid visual flash | ||
| textArea.style.position = "fixed" | ||
| textArea.style. left = "-999999px" | ||
| textArea.style.top = "-999999px" | ||
| document.body.appendChild(textArea) | ||
| textArea.focus() | ||
| textArea.select() | ||
|
|
||
| try { | ||
| // Note: execCommand is deprecated but remains the only option for non-secure contexts | ||
| document.execCommand("copy") | ||
| textArea.remove() | ||
| } catch (err) { | ||
| console.error("Fallback: Could not copy text", err) | ||
| textArea.remove() | ||
| throw err | ||
| } | ||
| } | ||
|
|
||
| trackEvent({ | ||
| name: event, | ||
| properties: { | ||
| name, | ||
| }, | ||
| }) | ||
| setHasCopied(true) | ||
| } catch (err) { | ||
| console.error("Failed to copy text: ", err) | ||
| } | ||
| }, | ||
| [event, name] | ||
| ) | ||
|
|
||
| return ( | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
|
|
@@ -41,16 +106,7 @@ export function ChartCopyButton({ | |
| "[&_svg]-h-3.5 h-7 w-7 rounded-[6px] [&_svg]:w-3.5", | ||
| className | ||
| )} | ||
| onClick={() => copyToClipboard(code)} | ||
|
There was a problem hiding this comment. Thank you for this refactoring, please could you add more details about the older-browsers and the non-secure contexts?
Contributor
Author
There was a problem hiding this comment. Thanks for the feedback! I've added detailed documentation explaining both paths: Modern path (
Fallback path (
I've added inline comments explaining the use cases for each path. There was a problem hiding this comment. Hmm, However execCommand is deprecated. |
||
| {...props} | ||
| > | ||
| <span className="sr-only">Copy</span> | ||
|
|
@@ -60,4 +116,4 @@ export function ChartCopyButton({ | |
| <TooltipContent className="bg-black text-white">Copy code</TooltipContent> | ||
| </Tooltip> | ||
| ) | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
We changed the array’s initial length from
12to9, but there’s no comment explaining the reason for this change.If the value is meant to be dynamic, we could derive it from
chartList.lengthinstead of hard-coding it.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.
Good point! The change from 12 to 9 was intentional to better match our scatter chart collection.
Context:
This PR adds 6 new scatter chart variants
The page previously rendered 12 total slots (actual charts + empty placeholders)
With only 6 scatter charts, this created 6 empty placeholder boxes, which looked sparse
Why 9:
Changing to 9 gives us a cleaner 3×3 grid on larger screens
Shows 6 actual scatter charts + 3 empty placeholders
The empty placeholders indicate room for future chart additions
Your suggestion about deriving from chartList.length:
I considered this, but the current implementation intentionally shows a few empty slots to indicate that more charts can be added. However, I'm open to either approach:
Keep it at 9 (current) - Shows intentional space for future charts
Use chartList.length - Dynamically adjusts, no empty slots
Use Math.max(chartList.length, 9) Shows charts + minimum 9 slots
Which approach would you prefer for consistency across the codebase?
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.
You hit it!
I noticed before these empty zones and slots and I was wondering why they were there.
The core team can decide on that.