Allow copy pasting components with CTRL+C and CTRL+V

Created on 19 July 2024, 5 months ago

Overview

As a content creator, I want to be able to copy and paste components so that I can conveniently use existing content as a basis of new content. This could work simply so that when user tries to copy a component, we place the underlying JSON structure in the clipboard. This may lead into error scenarios, like for example if the component had property expressions in its properties. In this scenario we should display the property expression as "missing". The user can either fill in it manually, or map it to another property.

User interface changes

Feature request
Status

Active

Component

Page builder

Created by

🇫🇮Finland lauriii Finland

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @lauriii
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    This may lead into error scenarios, like for example if the component had property expressions in its properties. In this scenario we should display the property expression as "missing". The user can either fill in it manually, or map it to another property.

    If the entire JSON structure is copied, I don't see how this could fail? 🤔

  • 🇫🇮Finland lauriii Finland

    This might be something we should give further consideration but I would imagine users would not expect that copying a component also copies the property expressions source data. This means that copying a component to a new host entity could lead into situation where property expression could be pointing to a target which doesn't exist in the new host entity.

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    This means that copying a component to a new host entity

    Ah, yes.

    StaticPropSources would copy over just fine, but not DynamicPropSources — for these, users should indeed be prompted.

    That's also what the DynamicPropSoruce::$expression is for: \Drupal\experience_builder\PropExpressions\StructuredData\StructuredDataPropExpressionInterface::isSupported() literally is designed to allow detecting this 👍

    IOW: the back end already has the necessary infrastructure.

  • 🇫🇮Finland lauriii Finland

    Adding this to the board as a potential delighter to add.

  • 🇺🇸United States freelock Seattle

    This would be huge -- and I see it as the single biggest advantage of Gutenberg over other Drupal solutions...

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
  • This is not a completely working MR thought but it duplicates the nodes and place it next to the selected node for now using the duplicate node reducers defined in layoutModelSlice.
    I have a bunch of questions here:-
    1) Say we select the component and copy that component with the use of Ctrl+C we will get the data related to that component inside copyComponent reducer.With the data copied along with the component to copy how do we define the place where we want to paste the component?For now we are using the duplicateNode reducers which places this next to the copied component.One probable solution that i can think of (might not be a good solution though) is to have spaces around the components in certain shapes which has a + Icon to add a component at that place.Because currently we cannot insert a component at desired location without dragging a component.
    2) Also i wanted a bit clarification of what all we need to copy.IMHO i think it makes sense to duplicate the whole node as we do in the duplicate node scenario and then let the user alter the props according to their needs.

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
    1. Very good question. Let's start with the simpler UX. Let's only allow CTRL+V when selecting a component or slot in the "layers" panel. Then:
      • if a slot is selected, append to the slot
      • if a component is selected, append to its first slot — and if there are no slots, alert('This component has no slots.');
      • if nothing is selected, alert('This component has no slots.');

      That won't be the final UX obviously (since there are no designs for this), but that's super MVP to be able to have a more detailed conversation!

    2. +1! 😊
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
  • So the current state of the MR does the following:-

    1. If we select a component and press Ctrl+C then it copies the component and if we press Ctrl+V then it pastes the component next to the selected component just like duplicate node does.
    2. If we select a component and press Ctrl+C then it copies the component and then we select another component after which we want to insert the component and the press Ctrl+V then it appends the copied component after the selected component.
  • I was trying to write a test case for this which is as follows:-

    it('Copy and paste a node using keyboard shortcuts', () => {
          cy.loadURLandWaitForXBLoaded();
    
          cy.getIframeBody()
            .find('[data-xb-uuid="dynamic-image-udf7d"]')
            .first()
            .click({ force: true });
          cy.get("body").type("{ctrl}C").wait(100);
          cy.getIframeBody()
            .find('[data-component-id="experience_builder:my-hero"]')
            .first()
            .click({ force: true });
          cy.get("body").type("{ctrl}V").wait(100);
          cy.pause()
    
          // Assert the position of the copied element.
    
        });

    I figured out that cypress tests does not support copy paste.You can see https://github.com/cypress-io/cypress/issues/2386 for more details.If anyone knows a better way to trigger copy paste with keyboard that will be really helpful.

  • Left some questions that i need clarification with.I'm not sure who is the right person to answer those but marking it NR.

  • First commit to issue fork.
  • Pipeline finished with Success
    3 months ago
    Total: 425s
    #294244
  • 🇺🇸United States ctrladel North Carolina, USA

    Took a look at this at the Barcelona contrib day. Found and fixed a bug with the logic for the keyboard shortcuts so copy/paste actually works now.

    I don't think reusing duplicateNode is going to be the right approach or at least it does not fully cover all the needed functionality. To be able to copy components between different nodes the component needs to be placed on the system clipboard and the json inserted into the tree. With the current approach it looks like we're only copying/pasting the component from app state.

  • 🇬🇧United Kingdom jessebaker

    As @ctrladel says in #1 I think the implementation here is more like duplicate with an extra step. It does not allow a user to, for example, copy a component, reload the page to remove other unwanted changes and then paste the component back in again. Or take a component from one node and paste it onto another.

    I think perhaps some of this work can be repurposed to make a CMD+D shortcut to allow for instant duplication of the selected component.

  • First commit to issue fork.
  • 🇺🇸United States bnjmnm Ann Arbor, MI

    Pushed the MR in a very WIP state so I'm switching it to draft- needs some cleaning up that I don't have time for atm, but it now works like a true copy paste. The original can be deleted but still pasted, and pasting can happen from one tab to the next.

    It is using localStorage, not clipboard, since clipboard only works with https and it's not uncommon to work on a non-https local. If we really want clipboard support I recommend doing it in a separate issue as we will need to determine if it is worth having two copypaste apis to support, and the possible confusion that might occur from pasting a stringified layout / model object into a document.

  • 🇫🇮Finland lauriii Finland

    The localStorage idea seems like a smart way to implement this so long as it works between tabs. 😊

    This is not in the scope of this issue but it would be great to be eventually able to copy and paste to XB from other apps. This could mean for example inserting text into a text element. I was wondering how would we differentiate between the state and the clipboard. Maybe we could achieve this by clearing the clipboard whenever we store XB clipboard in state. This way we could first check if the clipboard has content, and use it instead when that's the case.

    Should we handle CTRL+X here or move that too a follow-up?

  • 🇺🇸United States bnjmnm Ann Arbor, MI

    The tests now work - not quite ready to set to NR as we should test scenarios like pasting a component that has been deleted and if possible, pasting between tabs.

  • 🇺🇸United States bnjmnm Ann Arbor, MI
  • please add steps to reproduce how can i test it and where?

  • 🇺🇸United States hooroomoo

    There should also be copy and paste functionality added to the layers menu in a follow-up

  • 🇺🇸United States hooroomoo

    One comment

  • 🇬🇧United Kingdom jessebaker

    @bnjmnm is really busy with a bunch of other things at the moment. So if anyone else wants to push forward on this (implementing @hooroomoo's suggestion above) please feel free to take it on!

  • 🇺🇸United States hooroomoo

    Currently the issue is in the MR if a user copies and pastes within the iframe they can't do it consecutively because the it seems like the iframe doesn't receive the key events after the first time. So either the iframe would need to get focused again or the parent element outside the iframe needs the key listeners for copy and paste.

    But since 🌱 [Proposal] A different approach to the iFrame preview interactions Active gets rid of cross communication between iframe and the parent, I kinda want to wait till that's in since the way this is currently implemented will need to change anyway.

  • 🇺🇸United States hooroomoo

    Un-assigning myself for now in case anyone else wants to pick it up, get changes from 0.x first

  • 🇬🇧United Kingdom jessebaker

    I went ahead and resolved the conflicts on merging 0.x as it was quite tricket. Doing that has broken the new Cypress test because that still interacts with the iFrame in the 'old' way.

    I think going through and updating the Cypress test to replace e.g. cy.getIframeBody().findAllByText('XB Needs This For The Time Being').should('have.length', 2); with cy.clickComponentInPreview('Hero', 1) will get the test working again.

    Leaving unassigned if anyone wants to pick up, else I can jump on this tomorrow.

  • 🇬🇧United Kingdom jessebaker

    This is currently working with one minor visual issue that looks pretty bad. After pasting the overlay/outline UI flashes up, unstyled/broken.

    I think this is ready for review, but I will continue looking into this last visual issue. Depending on the scale of the fix, I will either push a further commit to this branch or create a new issue.

  • 🇺🇸United States hooroomoo

    Works great! Works between tabs and also from layers menu too. Just a couple of comments

  • Pipeline finished with Skipped
    about 2 months ago
    #327262
  • Pipeline finished with Skipped
    about 2 months ago
    #327263
  • Status changed to Fixed about 1 month ago
  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024