-
Notifications
You must be signed in to change notification settings - Fork 16
feat(code-editor): implement copy button for editor content #3768
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
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThe pull request adds a copy-to-clipboard feature to the CodeEditor component, including layout changes to position a copy button, a new translationLanguage prop for internationalization, snackbar feedback upon successful copy, and translation strings across eight languages (Danish, German, English, Finnish, French, Dutch, Norwegian, and Swedish). Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 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 |
|
Documentation has been published to https://lundalogik.github.io/lime-elements/versions/PR-3768/ |
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.
Actionable comments posted: 4
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (10)
src/components/code-editor/code-editor.scss(2 hunks)src/components/code-editor/code-editor.tsx(6 hunks)src/translations/da.ts(1 hunks)src/translations/de.ts(1 hunks)src/translations/en.ts(1 hunks)src/translations/fi.ts(1 hunks)src/translations/fr.ts(1 hunks)src/translations/nl.ts(1 hunks)src/translations/no.ts(1 hunks)src/translations/sv.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
**/*.{ts,tsx}: Imports from other files in the same module (lime-elements) must use relative paths. Using absolute paths for imports will cause the production build to fail.
Files:
src/translations/nl.tssrc/translations/no.tssrc/translations/fr.tssrc/translations/sv.tssrc/translations/fi.tssrc/components/code-editor/code-editor.tsxsrc/translations/da.tssrc/translations/en.tssrc/translations/de.ts
src/components/**/*.scss
📄 CodeRabbit inference engine (.cursor/rules/css-class-names-in-components-using-shadow-dom.mdc)
Do not use BEM-style class names in CSS for components, as components use shadow-DOM.
Files:
src/components/code-editor/code-editor.scss
**/*.{tsx,scss}
⚙️ CodeRabbit configuration file
**/*.{tsx,scss}: Almost all our components use shadow-DOM. Therefore, we have no need of BEM-style class names in our CSS.
Files:
src/components/code-editor/code-editor.scsssrc/components/code-editor/code-editor.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/wrap-multiple-jsx-elements-in-host-not-in-array.mdc)
When returning multiple JSX elements from the
rendermethod, never wrap them in an array literal. Instead, always wrap them in the special<Host>element. When there is already a single top-level element, it does not have to be wrapped in<Host>.
Files:
src/components/code-editor/code-editor.tsx
⚙️ CodeRabbit configuration file
**/*.tsx: Our.tsxfiles are typically using StencilJS, not React. When a developer wants to return multiple top-level JSX elements from therendermethod, they will sometimes wrap them in an array literal. In these cases, rather than recommending they addkeyproperties to the elements, recommend removing the hardcoded array literal. Recommend wrapping the elements in StencilJS's special<Host>element.
Files:
src/components/code-editor/code-editor.tsx
src/components/**/*.tsx
⚙️ CodeRabbit configuration file
src/components/**/*.tsx: When contributors add new props to existing components in the lime-elements repository, they should always add documentation examples that demonstrate the new prop's usage and explain how it works. This helps with user adoption, feature discoverability, and maintains comprehensive documentation.
Files:
src/components/code-editor/code-editor.tsx
🧠 Learnings (10)
📚 Learning: 2025-04-17T09:39:36.254Z
Learnt from: adrianschmidt
Repo: Lundalogik/lime-elements PR: 3530
File: src/components/text-editor/examples/text-editor-composite.tsx:4-7
Timestamp: 2025-04-17T09:39:36.254Z
Learning: For lime-elements, example files should import types from the public API using 'limetech/lime-elements' rather than defining duplicate types locally. This includes types like `EditorUiType` which are part of the component's public API.
Applied to files:
src/components/code-editor/code-editor.scss
📚 Learning: 2025-04-29T14:15:32.104Z
Learnt from: adrianschmidt
Repo: Lundalogik/lime-elements PR: 3529
File: src/components/ai-avatar/ai-avatar.scss:20-22
Timestamp: 2025-04-29T14:15:32.104Z
Learning: In components using Shadow DOM (like those in lime-elements), the universal selector `*` is already scoped to the component's shadow tree and won't cause unwanted side effects outside the component. There's no functional difference between `*` and `:host *` in this context.
Applied to files:
src/components/code-editor/code-editor.scss
📚 Learning: 2025-08-07T14:39:00.053Z
Learnt from: Kiarokh
Repo: Lundalogik/lime-elements PR: 3638
File: src/components/radio-button-group/radio-button-group.tsx:32-32
Timestamp: 2025-08-07T14:39:00.053Z
Learning: The `limel-radio-button-group` component uses `shadow: false` because it's a pure wrapper around `limel-list` with no component-specific styles, following the pattern of other wrapper components like `limel-action-bar-overflow-menu`.
Applied to files:
src/components/code-editor/code-editor.scss
📚 Learning: 2025-04-14T12:22:22.298Z
Learnt from: Kiarokh
Repo: Lundalogik/lime-elements PR: 3518
File: src/components/chip-set/chip-set.tsx:367-385
Timestamp: 2025-04-14T12:22:22.298Z
Learning: In limel-chip-set, the onClick handler should be placed on the content div (<div slot="content">) rather than on the limel-notched-outline component, and should only be applied when type is 'input'.
Applied to files:
src/components/code-editor/code-editor.scss
📚 Learning: 2025-10-12T21:44:29.530Z
Learnt from: adrianschmidt
Repo: Lundalogik/lime-elements PR: 3703
File: context7.json:24-24
Timestamp: 2025-10-12T21:44:29.530Z
Learning: In the lime-elements repository, CSS custom properties follow a two-tier naming convention: `--lime-*` properties are public API intended for external consumers and are documented in theming guides (e.g., `--lime-primary-color`, `--lime-on-primary-color`), while `--limel-*` properties are internal implementation details used as fallbacks and are not documented for public use (e.g., `--limel-theme-primary-color`). The `--lime-` prefix is used specifically to avoid naming conflicts with third-party code.
Applied to files:
src/components/code-editor/code-editor.scss
📚 Learning: 2025-09-03T07:17:23.267Z
Learnt from: Kiarokh
Repo: Lundalogik/lime-elements PR: 3658
File: src/components/profile-picture/profile-picture.scss:120-124
Timestamp: 2025-09-03T07:17:23.267Z
Learning: Issue #3666 in the lime-elements repository tracks the removal of a CSS hack in profile-picture component that overrides icon color via --limel-theme-on-surface-color, to be replaced with proper icon color support in limel-icon-button.
Applied to files:
src/components/code-editor/code-editor.scss
📚 Learning: 2025-09-03T07:17:20.420Z
Learnt from: Kiarokh
Repo: Lundalogik/lime-elements PR: 3658
File: src/components/profile-picture/profile-picture.scss:7-16
Timestamp: 2025-09-03T07:17:20.420Z
Learning: In Lime Elements, CSS custom property naming follows a specific convention: properties starting with `--limel-` are for internal use only, while properties starting with the component name (e.g., `--profile-picture-border-radius`) are publicly documented and intended for external consumers. Public properties should be documented with JSDoc comments.
Applied to files:
src/components/code-editor/code-editor.scss
📚 Learning: 2025-04-14T10:01:18.721Z
Learnt from: Kiarokh
Repo: Lundalogik/lime-elements PR: 3518
File: src/components/notched-outline/examples/notched-outline-basic.scss:11-22
Timestamp: 2025-04-14T10:01:18.721Z
Learning: Examples in Lime Elements use Shadow DOM (with `shadow: true` in the component decorator) for style encapsulation, making additional class-based selectors unnecessary. This ensures styles are automatically scoped to each component without leaking to the rest of the application.
Applied to files:
src/components/code-editor/code-editor.scss
📚 Learning: 2025-07-30T14:20:41.791Z
Learnt from: Kiarokh
Repo: Lundalogik/lime-elements PR: 3624
File: src/components/button-group/button-group.scss:115-118
Timestamp: 2025-07-30T14:20:41.791Z
Learning: In lime-elements, CSS custom properties like `--limel-theme-surface-background-color` are applied at :root level via `core-styles.scss` and should be expected to be available without fallbacks. The library's architecture guarantees these theme variables are present, so adding fallbacks would be redundant and go against the design intention.
Applied to files:
src/components/code-editor/code-editor.scss
📚 Learning: 2025-04-25T14:22:02.157Z
Learnt from: Kiarokh
Repo: Lundalogik/lime-elements PR: 3529
File: src/components/lime-ai-avatar/lime-ai-avatar.scss:24-28
Timestamp: 2025-04-25T14:22:02.157Z
Learning: In lime-elements, CSS custom properties follow a specific naming convention:
1. Internal use variables (not for consumers) are prefixed with `--limel-component-name-`
2. External use variables (intended for consumers) are prefixed with `--component-name-`
Variables documented with JSDoc comments are typically meant for external use.
Applied to files:
src/components/code-editor/code-editor.scss
🧬 Code graph analysis (1)
src/components/code-editor/code-editor.tsx (1)
src/components/date-picker/date.types.ts (1)
Languages(16-25)
🪛 Biome (2.1.2)
src/components/code-editor/code-editor.tsx
[error] 365-371: Static Elements should not be interactive.
To add interactivity such as a mouse or key event listener to a static element, give the element an appropriate role value.
(lint/a11y/noStaticElementInteractions)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Docs / Publish Docs
- GitHub Check: Build
- GitHub Check: Test
🔇 Additional comments (13)
src/translations/en.ts (1)
15-16: LGTM! Translation keys are well-defined.The English translation keys for the copy-to-clipboard feature are clear and follow the established naming convention.
src/translations/da.ts (1)
15-16: LGTM! Danish translations added.The Danish translations align with the copy-to-clipboard feature structure.
src/translations/nl.ts (1)
15-16: LGTM! Dutch translations added.The Dutch translations for the code editor copy functionality are properly structured.
src/components/code-editor/code-editor.scss (1)
124-124: LGTM! Host positioning enables absolute child placement.Adding
position: relativeto the host is appropriate for containing the absolutely positioned copy button.src/translations/fr.ts (1)
15-16: LGTM! French translations added.The French translations are properly integrated for the copy-to-clipboard feature.
src/translations/no.ts (1)
15-16: LGTM! Norwegian translations added.The Norwegian translations follow the established structure for the code editor copy functionality.
src/translations/de.ts (1)
15-16: LGTM! German translations added.The German translations are properly structured for the copy-to-clipboard feature.
src/translations/sv.ts (1)
15-16: LGTM! Swedish translations added.The Swedish translations complete the localization for the copy-to-clipboard feature.
src/translations/fi.ts (1)
15-16: LGTM!The Finnish translation strings for the copy functionality are correctly added and follow the existing structure.
src/components/code-editor/code-editor.tsx (4)
151-152: LGTM!The
isOpenstate is properly initialized and used to control the snackbar visibility.
387-391: LGTM!The snackbar implementation correctly uses the translated message and is properly controlled by the
isOpenstate.
423-425: LGTM!The snackbar hide handler is correctly implemented.
28-29: Consider relocating theLanguagestype to a shared location.While the relative import paths are correct per the coding guidelines, importing
Languagesfrom the date-picker component creates unnecessary coupling between code-editor and date-picker. Consider moving this shared type to a common types file (e.g.,src/global/types.tsorsrc/types/languages.ts) for better modularity.⛔ Skipped due to learnings
Learnt from: adrianschmidt Repo: Lundalogik/lime-elements PR: 3530 File: src/components/text-editor/examples/text-editor-composite.tsx:4-7 Timestamp: 2025-04-17T09:39:36.254Z Learning: For lime-elements, example files should import types from the public API using 'limetech/lime-elements' rather than defining duplicate types locally. This includes types like `EditorUiType` which are part of the component's public API.Learnt from: Kiarokh Repo: Lundalogik/lime-elements PR: 3530 File: src/components/text-editor/examples/text-editor-composite.tsx:4-7 Timestamp: 2025-04-16T14:14:18.253Z Learning: For lime-elements, example files should be self-contained and avoid importing internal implementation details. This includes not importing types from internal files like '../types.ts', especially those marked with beta. Duplicating simple type definitions in example files is preferred over importing internal types that may change.Learnt from: adrianschmidt Repo: Lundalogik/lime-elements PR: 3464 File: src/components/text-editor/prosemirror-adapter/plugins/image/inserter.ts:1-11 Timestamp: 2025-03-04T13:48:44.712Z Learning: The lime-elements codebase must use relative imports (even with multiple "../") rather than alias imports or re-exports because they export types to `<repo-root>/dist/types`. Using non-relative imports breaks type generation as import paths from generated type files point to full .ts files rather than .d.ts files.Learnt from: Kiarokh Repo: Lundalogik/lime-elements PR: 3530 File: src/components/text-editor/examples/text-editor-composite.tsx:4-7 Timestamp: 2025-04-16T14:14:18.253Z Learning: For lime-elements, example files should be self-contained and avoid importing internal implementation details. Example files should only import public exports from 'limetech/lime-elements' or use relative imports for other files within the examples folder. Duplicating simple type definitions in example files is preferred over importing internal types.
62c22bc to
0789e08
Compare
0f35412 to
b99ca15
Compare
LucyChyzhova
left a comment
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.
Great job!! 🚀 Looks super nice! 😍
b99ca15 to
e5f33c7
Compare
e5f33c7 to
0aa0445
Compare
0aa0445 to
ab7f4a4
Compare
Kiarokh
left a comment
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.
Really great work and awesome feature! Love it. Please just apply my suggestions, as described in the comments below, to make this 100% bulletproof
6a1cfcf to
0d513ed
Compare
| private getButtonAriaLabel() { | ||
| const editorLabel = this.label || 'code editor'; | ||
| const copyLabel = translate.get( | ||
| 'code-editor.copy', | ||
| this.translationLanguage | ||
| ); | ||
| const copiedLabel = translate.get( | ||
| 'code-editor.copied', | ||
| this.translationLanguage | ||
| ); | ||
| const failedToCopyLabel = translate.get( | ||
| 'code-editor.copy-failed', | ||
| this.translationLanguage | ||
| ); | ||
| if (this.wasCopied === 'success') { | ||
| return `${copiedLabel} ${editorLabel}`; | ||
| } | ||
| if (this.wasCopied === 'failed') { | ||
| return `${failedToCopyLabel} ${editorLabel}`; | ||
| } | ||
| return `${copyLabel} ${editorLabel}`; | ||
| } | ||
|
|
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.
This code would generate aria-labels which aren't super nice. A better design would be having the label as a variable of the translation key; which I what I was trying to say with my comment previous, but I didn't clarify. Sorry for that.
The only thing would be trying to have a fallback for when the label is not set by the consumer.
See my fixup: cd4291d
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.
- If the code editor has a label prop (like "JSON Configuration"), use that
- Otherwise, use the translated word for "code editor" in the current language
cd4291d to
e816b36
Compare
|
🎉 This PR is included in version 38.36.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
fix: #3769
Summary by CodeRabbit
New Features
Localization
✏️ Tip: You can customize this high-level summary in your review settings.
Review:
Browsers tested:
(Check any that applies, it's ok to leave boxes unchecked if testing something didn't seem relevant.)
Windows:
Linux:
macOS:
Mobile: