issue/3857/feat/copying-distributions#4538
Open
lsabor wants to merge 3 commits into
Open
Conversation
closes #3859 implements copy and paste from arbitrary distributions support ctrl+c and ctrl+v
Contributor
📝 WalkthroughWalkthroughThis PR adds localized distribution clipboard messages and implements copy/paste support for continuous forecast distributions, including validation, keyboard shortcuts, toast feedback, and parent-state integration. ChangesDistribution clipboard flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
actor User
participant Menu as ContinuousClipboardMenu
participant Clipboard as Browser Clipboard
participant State as Forecast State
User->>Menu: Copy distribution or press Ctrl/Cmd+C
Menu->>Clipboard: Write serialized distribution
Clipboard-->>Menu: Success or error
Menu->>User: Show copy result toast
User->>Menu: Paste distribution or press Ctrl/Cmd+V
Menu->>Clipboard: Read clipboard text
Clipboard-->>Menu: Return clipboard text
Menu->>Menu: Parse and validate distribution
Menu->>State: onPaste(type, components)
State->>State: Update components and input mode
Menu->>User: Show paste result toast
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@front_end/messages/zh-TW.json`:
- Around line 2081-2086: Update the translation values for the keys
"distributionCopied", "distributionCopyFailed", "distributionPasted",
"distributionPasteInvalid", "copyDistribution", and "pasteDistribution" to use
「分佈」 instead of 「分配」 so the terminology matches the forecasting context; locate
these string values in front_end/messages/zh-TW.json (the keys listed above) and
replace each occurrence of 「分配」 with 「分佈」 while preserving the rest of the
messages.
In
`@front_end/src/components/forecast_maker/continuous_input/continuous_clipboard_menu.tsx`:
- Around line 24-36: The isClipboardDistribution type guard currently checks
marker and type but must also validate that (data as
ClipboardDistribution).components is an array and that each component has the
required shape: ensure components is Array.isArray(...) and for each element
verify a discriminant (e.g., component.type or similar) and then for slider
components confirm presence and correct types for weight, left, center, right,
and for quantile components confirm presence and correct type for quantile; keep
referencing CLIPBOARD_MARKER, VALID_INPUT_TYPES and the ClipboardDistribution
shape and update isClipboardDistribution to return false for any malformed
component to prevent downstream runtime errors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2628ab1d-d8ec-4edf-b54a-c454fc13fba0
📒 Files selected for processing (11)
front_end/messages/cs.jsonfront_end/messages/en.jsonfront_end/messages/es.jsonfront_end/messages/pt.jsonfront_end/messages/zh-TW.jsonfront_end/messages/zh.jsonfront_end/src/components/forecast_maker/continuous_input/continuous_clipboard_menu.tsxfront_end/src/components/forecast_maker/continuous_input/continuous_input_container.tsxfront_end/src/components/forecast_maker/continuous_input/index.tsxfront_end/src/components/forecast_maker/forecast_maker_group/continuous_input_wrapper.tsxfront_end/src/components/forecast_maker/forecast_maker_question/forecast_maker_continuous.tsx
Comment on lines
+2081
to
+2086
| "distributionCopied": "分配已複製到剪貼簿", | ||
| "distributionCopyFailed": "複製分配失敗", | ||
| "distributionPasted": "從剪貼簿貼上了分配", | ||
| "distributionPasteInvalid": "剪貼簿中未找到有效的分配", | ||
| "copyDistribution": "將分配複製到剪貼簿", | ||
| "pasteDistribution": "從剪貼簿貼上分配", |
Contributor
There was a problem hiding this comment.
Use 「分佈」 instead of 「分配」 for consistency and correctness.
In this locale file, distribution in forecasting context is already translated as 「分佈」. Using 「分配」 here introduces inconsistent terminology and reads as “allocation.”
Suggested wording update
- "distributionCopied": "分配已複製到剪貼簿",
- "distributionCopyFailed": "複製分配失敗",
- "distributionPasted": "從剪貼簿貼上了分配",
- "distributionPasteInvalid": "剪貼簿中未找到有效的分配",
- "copyDistribution": "將分配複製到剪貼簿",
- "pasteDistribution": "從剪貼簿貼上分配",
+ "distributionCopied": "分佈已複製到剪貼簿",
+ "distributionCopyFailed": "複製分佈失敗",
+ "distributionPasted": "已從剪貼簿貼上分佈",
+ "distributionPasteInvalid": "剪貼簿中未找到有效的分佈",
+ "copyDistribution": "將分佈複製到剪貼簿",
+ "pasteDistribution": "從剪貼簿貼上分佈",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "distributionCopied": "分配已複製到剪貼簿", | |
| "distributionCopyFailed": "複製分配失敗", | |
| "distributionPasted": "從剪貼簿貼上了分配", | |
| "distributionPasteInvalid": "剪貼簿中未找到有效的分配", | |
| "copyDistribution": "將分配複製到剪貼簿", | |
| "pasteDistribution": "從剪貼簿貼上分配", | |
| "distributionCopied": "分佈已複製到剪貼簿", | |
| "distributionCopyFailed": "複製分佈失敗", | |
| "distributionPasted": "已從剪貼簿貼上分佈", | |
| "distributionPasteInvalid": "剪貼簿中未找到有效的分佈", | |
| "copyDistribution": "將分佈複製到剪貼簿", | |
| "pasteDistribution": "從剪貼簿貼上分佈", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@front_end/messages/zh-TW.json` around lines 2081 - 2086, Update the
translation values for the keys "distributionCopied", "distributionCopyFailed",
"distributionPasted", "distributionPasteInvalid", "copyDistribution", and
"pasteDistribution" to use 「分佈」 instead of 「分配」 so the terminology matches the
forecasting context; locate these string values in front_end/messages/zh-TW.json
(the keys listed above) and replace each occurrence of 「分配」 with 「分佈」 while
preserving the rest of the messages.
Contributor
🚀 Preview EnvironmentYour preview environment is ready!
Details
ℹ️ Preview Environment InfoIsolation:
Limitations:
Cleanup:
|
lsabor
changed the title
…/feat/copying-distributions
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@front_end/messages/zh-TW.json`:
- Line 1953: Update the tournamentTimelineClosed translation in the zh-TW locale
to use Traditional Chinese characters, changing the value from “待处理的决议” to
“待處理的決議” while leaving the key and surrounding translations unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6c16e339-edb4-4bc5-80d5-cdafa2961d1b
📒 Files selected for processing (6)
front_end/messages/cs.jsonfront_end/messages/en.jsonfront_end/messages/es.jsonfront_end/messages/pt.jsonfront_end/messages/zh-TW.jsonfront_end/messages/zh.json
🚧 Files skipped from review as they are similar to previous changes (4)
- front_end/messages/en.json
- front_end/messages/es.json
- front_end/messages/cs.json
- front_end/messages/zh.json
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@front_end/messages/zh-TW.json`:
- Line 1953: Update the tournamentTimelineClosed translation in the zh-TW locale
to use Traditional Chinese characters, changing the value from “待处理的决议” to
“待處理的決議” while leaving the key and surrounding translations unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6c16e339-edb4-4bc5-80d5-cdafa2961d1b
📒 Files selected for processing (6)
front_end/messages/cs.jsonfront_end/messages/en.jsonfront_end/messages/es.jsonfront_end/messages/pt.jsonfront_end/messages/zh-TW.jsonfront_end/messages/zh.json
🚧 Files skipped from review as they are similar to previous changes (4)
- front_end/messages/en.json
- front_end/messages/es.json
- front_end/messages/cs.json
- front_end/messages/zh.json
🛑 Comments failed to post (1)
front_end/messages/zh-TW.json (1)
1953-1953: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use Traditional Chinese characters.
The string
"待处理的决议"uses Simplified Chinese characters (处理,决议). Since this is the Traditional Chinese (zh-TW) locale file, it should use Traditional characters (待處理的決議).📝 Proposed fix
- "tournamentTimelineClosed": "待处理的决议", + "tournamentTimelineClosed": "待處理的決議",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements."tournamentTimelineClosed": "待處理的決議",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@front_end/messages/zh-TW.json` at line 1953, Update the tournamentTimelineClosed translation in the zh-TW locale to use Traditional Chinese characters, changing the value from “待处理的决议” to “待處理的決議” while leaving the key and surrounding translations unchanged.
- Skip the Ctrl+C/Ctrl+V shortcut handling when focus is on a form field (e.g. a percentile input), so native copy/paste isn't hijacked there. - Skip the slider<->table mode-sync effect when a mode change comes from a paste, since the pasted components are already correct and shouldn't be overwritten by deriving them from the other (unrelated) input mode. - Store copied quantile values in a scale-independent internal [0,1] location (like slider components already do) instead of the source question's real-world units, and rescale on paste so distributions transfer correctly between questions with different ranges. - Reconcile pasted quantile values with the target question's own open/closed bounds: clamp bound-probability values to 0 on a closed bound (matching validation, since there's no UI to fix it otherwise) or to [0.1, 99.9] on an open bound, and nudge q1/q2/q3 strictly inside a closed bound. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
front_end/src/components/forecast_maker/continuous_input/continuous_clipboard_menu.tsx (2)
208-217: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHandle uppercase keys for shortcuts.
When Caps Lock is enabled,
e.keywill be"C"or"V", causing the exact shortcut check to fail. Converte.keyto lowercase to ensure the shortcut works regardless of the user's Caps Lock state.💡 Proposed fix
- if (e.key === "c") { + const key = e.key.toLowerCase(); + if (key === "c") { // Only intercept if no text is selected const selection = window.getSelection()?.toString(); if (selection) return; e.preventDefault(); handleCopy(); - } else if (e.key === "v" && !disabled) { + } else if (key === "v" && !disabled) { e.preventDefault(); handlePaste(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@front_end/src/components/forecast_maker/continuous_input/continuous_clipboard_menu.tsx` around lines 208 - 217, Update the keyboard shortcut checks in the continuous clipboard menu handler to compare a lowercase-normalized e.key, ensuring copy and paste work with Caps Lock enabled while preserving the existing selection and disabled checks.
113-123: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winStrengthen validation to include component structure checks.
The
isClipboardDistributiontype guard validates the marker and type but does not verify the structure ofcomponents. Malformed clipboard data could cause runtime errors downstream when:
componentsis not an array (.map()will fail)- Slider components are missing required fields (
weight,left,center,right)- Quantile components are missing the
quantilefield🛡️ Proposed fix to add component structure validation
+function isValidSliderComponent(c: unknown): c is DistributionSliderComponent { + return ( + typeof c === "object" && + c !== null && + typeof (c as DistributionSliderComponent).weight === "number" && + typeof (c as DistributionSliderComponent).left === "number" && + typeof (c as DistributionSliderComponent).center === "number" && + typeof (c as DistributionSliderComponent).right === "number" + ); +} + +function isValidQuantileComponent(c: unknown): c is DistributionQuantileComponent[number] { + return ( + typeof c === "object" && + c !== null && + "quantile" in c + ); +} + function isClipboardDistribution(data: unknown): data is ClipboardDistribution { - return ( + if (!( typeof data === "object" && data !== null && CLIPBOARD_MARKER in data && (data as ClipboardDistribution)[CLIPBOARD_MARKER] === true && "type" in data && "components" in data && VALID_INPUT_TYPES.includes((data as ClipboardDistribution).type) - ); + )) { + return false; + } + + const { type, components } = data as ClipboardDistribution; + if (!Array.isArray(components) || components.length === 0) { + return false; + } + + if (type === ContinuousForecastInputType.Slider) { + return components.every(isValidSliderComponent); + } + return components.every(isValidQuantileComponent); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@front_end/src/components/forecast_maker/continuous_input/continuous_clipboard_menu.tsx` around lines 113 - 123, Strengthen the isClipboardDistribution type guard by validating components is an array and checking each component according to its type: slider components must contain weight, left, center, and right, while quantile components must contain quantile. Preserve the existing marker and valid input type checks, and reject malformed clipboard data before it reaches downstream processing.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@front_end/src/components/forecast_maker/continuous_input/continuous_clipboard_menu.tsx`:
- Around line 208-217: Update the keyboard shortcut checks in the continuous
clipboard menu handler to compare a lowercase-normalized e.key, ensuring copy
and paste work with Caps Lock enabled while preserving the existing selection
and disabled checks.
- Around line 113-123: Strengthen the isClipboardDistribution type guard by
validating components is an array and checking each component according to its
type: slider components must contain weight, left, center, and right, while
quantile components must contain quantile. Preserve the existing marker and
valid input type checks, and reject malformed clipboard data before it reaches
downstream processing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 892c1724-c6aa-42f1-8b52-e6f36d12a11c
📒 Files selected for processing (5)
front_end/src/components/forecast_maker/continuous_input/continuous_clipboard_menu.tsxfront_end/src/components/forecast_maker/continuous_input/continuous_input_container.tsxfront_end/src/components/forecast_maker/continuous_input/index.tsxfront_end/src/components/forecast_maker/forecast_maker_group/continuous_input_wrapper.tsxfront_end/src/components/forecast_maker/forecast_maker_question/forecast_maker_continuous.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- front_end/src/components/forecast_maker/forecast_maker_group/continuous_input_wrapper.tsx
- front_end/src/components/forecast_maker/forecast_maker_question/forecast_maker_continuous.tsx
- front_end/src/components/forecast_maker/continuous_input/continuous_input_container.tsx
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
closes #3857
implements copy and paste from arbitrary distributions
support ctrl+c and ctrl+v
Summary by CodeRabbit
Summary of changes
New Features
Localization