Skip to content

Layer copy pasting #1791

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

Closed
wants to merge 8 commits into from
Closed

Conversation

SoloDevAbu
Copy link
Contributor

@SoloDevAbu SoloDevAbu commented Apr 16, 2025

Description

fixes Copy-pasting a layer unexpectedly duplicates it

Related Issues

fixes #1723

  • Bug fix
  • New feature
  • Documentation update
  • Release
  • Refactor
  • Other (please describe):

Testing

Tested locally

Screenshots

Screencast.from.2025-04-16.22-34-00.webm

Important

Fixes layer duplication bug, adds website crawling feature, and improves error handling and UI in Onlook Studio.

  • Behavior:
    • Fixes layer duplication issue in CopyManager by ensuring the copied state is restored after duplication.
    • Adds website crawling feature using CrawlerService in crawler.ts.
    • Updates PromptingCard.tsx to handle URL input for crawling and display results.
  • Error Handling:
    • Improves error handling in CodeManager and ElementManager by adding try-catch blocks and error messages.
    • Adds toast notifications for errors in CopyManager and PageTreeNode.tsx.
  • UI and Localization:
    • Updates translation.json and messages/en.json to include new strings for crawling feature.
    • Modifies PageTreeNode.tsx to handle page duplication with a new naming convention.
    • Adds new dependencies in package.json for @mendable/firecrawl-js.

This description was created by Ellipsis for f54c471. It will automatically update as commits are pushed.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Changes requested. Reviewed everything up to f54c471 in 3 minutes and 1 seconds

More details
  • Looked at 504 lines of code in 11 files
  • Skipped 0 files when reviewing.
  • Skipped posting 14 drafted comments based on config settings.
1. apps/studio/.env.example:8
  • Draft comment:
    Added Firecrawl API key placeholder. Ensure that valid keys are provided and sensitive data is not exposed in production.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
2. apps/studio/package.json:43
  • Draft comment:
    Added '@mendable/firecrawl-js' dependency for Firecrawl integration. Confirm compatibility with other dependencies.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
3. apps/studio/src/lib/editor/engine/code/index.ts:67
  • Draft comment:
    Improved error handling in getCodeBlock using try/catch solidly prevents crashes when retrieving code blocks fails.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
4. apps/studio/src/lib/editor/engine/copy/index.ts:163
  • Draft comment:
    Typo in duplicate(): should check 'this.copied' instead of 'this.copy' to verify that the copied element exists.
  • Reason this comment was not posted:
    Marked as duplicate.
5. apps/studio/src/lib/editor/engine/element/index.ts:139
  • Draft comment:
    Enhanced error handling in element deletion using try/catch with toast notifications improves user feedback.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
6. apps/studio/src/lib/projects/create.ts:92
  • Draft comment:
    sendPrompt() now includes a 'crawledContent' parameter. Verify that the backend channel properly handles this new parameter.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
7. apps/studio/src/lib/services/crawler.ts:25
  • Draft comment:
    New CrawlerService encapsulates Firecrawl integration effectively. Ensure proper API key loading and error messaging.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
8. apps/studio/src/locales/en/translation.json:71
  • Draft comment:
    New 'crawl' translation keys added for the crawl feature. Confirm consistency with the UI and other language files.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
9. apps/studio/src/routes/editor/LayersPanel/Tree/PageTreeNode.tsx:79
  • Draft comment:
    Page duplication logic now uses regex to update the page name. Ensure this handles edge cases (e.g., single-segment paths) as expected.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50%
    None
10. apps/studio/src/routes/projects/PromptCreation/PromptingCard.tsx:199
  • Draft comment:
    Typo in toast title: 'URl crawled' should be 'URL crawled'.
  • Reason this comment was not posted:
    Marked as duplicate.
11. apps/web/client/messages/en.json:67
  • Draft comment:
    Updated translations include new crawl feature keys; ensure placeholder formatting and keys match those in the studio translations.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50%
    None
12. apps/studio/src/lib/editor/engine/copy/index.ts:163
  • Draft comment:
    Typographical/lexical issue: The condition 'if (!this.copy)' seems incorrect because 'copy' is a function and not the copied content. It likely should be checking 'if (!this.copied)' to verify if any content was copied.
  • Reason this comment was not posted:
    Marked as duplicate.
13. apps/studio/src/routes/projects/PromptCreation/PromptingCard.tsx:200
  • Draft comment:
    Typo: The toast title 'URl crawled' should have 'URL' in all uppercase. Please change it to 'URL crawled'.
  • Reason this comment was not posted:
    Marked as duplicate.
14. apps/web/client/messages/en.json:71
  • Draft comment:
    Typo detected: In the placeholder text, 'https:://' should be corrected to 'https://'.
  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_LsmjCL2l1nmoCebh


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

this.copied = savedCopied;
try {
await this.copy();
if (!this.copy) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: In duplicate(), check this.copied instead of this.copy (a method reference), else the check will always be truthy.

Suggested change
if (!this.copy) {
if (!this.copied) {

SoloDevAbu and others added 4 commits April 17, 2025 00:21
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
Fixed the typo

Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
Fixed typo

Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
@spartan-uyen
Copy link

Hey, thanks for checking.
I did a quick review of your PR and I still encountered an unexpected duplication of elements when performing both copy/paste actions using key presses or clicking the "Duplicate" option. Here's my video:
https://github.com/user-attachments/assets/2ac207bc-cf07-4f21-97e5-4c4c8e392bf4

@SoloDevAbu
Copy link
Contributor Author

SoloDevAbu commented Apr 17, 2025

Thanks for your feedback. The issue is much complex than what i thought would be. I am looking for the actual reason for this issue.
Can you please check the other PR of mine which are for the issues #1777 and #1781
Specially the PR for adding Firecrawl to crawl any website. Can you please paste any site link and write the prompt to create a similar website.
If any modifications need in the firecrawl PR then please let me know.

@SoloDevAbu SoloDevAbu closed this Apr 17, 2025
@SoloDevAbu SoloDevAbu deleted the layer-copy-pasting branch May 12, 2025 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bug][Layers] Copy-pasting a layer unexpectedly duplicates it
2 participants