Add 'Duplicate' action to elements. #770

JoshuaCarter · 2020-01-09T06:19:42Z

Duplicates element.
Adds ' (Copy)' to the Title.
Reorder duplicate to position after the cloned element.

We're currently on 4.0.5 so I've PR'd this to the 4.0 branch. I tried a quick cherry-pick onto the 4 branch but it didn't work right off the bat and I'm lazy 😪

Direction/feedback welcome.

ScopeyNZ · 2020-01-09T18:23:13Z

Nice! Awesome to see some more contributions to this React stuff. Hopefully you found it actually kind of fun and easy?

This will need to be based on the 4 branch, as it's a minor feature it'll have to go into Elemental 4.4. You should be able to do this with:

git rebase HEAD~1 --onto origin/4

Assuming that you're checked out on the PR branch and "origin" in this case is a remote with an up to date copy of `4 (preferably origin is just this repo). You might have to solve conflicts with the rebase.

Additionally, the tests are failing as you need to ensure you commit the dist files for the JavaScript. Just a yarn build and commit whatever is changed.

I don't have time to check this out and play around sorry, I'll leave that to one of the SSLtd team.

brynwhyman · 2020-01-09T22:32:10Z

Awesome! There's actually an open issue for this requirement that includes some light requirements from the Silverstripe UX team (I've just asked them to make the design link public).

#562

JoshuaCarter · 2020-01-10T00:43:33Z

Hey @ScopeyNZ and @brynwhyman, a few points/questions:

I've moved the 'Duplicate' option before 'Archive'.
Code now adds ' copy {optional number starting with 2}' rather than multiples of ' (Copy)'.
I'm doing this for a client (on their dime) and they're using 4.0.5 which is why I've targeted the 4.0 branch. So I can't necessarily justify the additional work to make this work on the 4 branch (including animation/alert which aren't on 4.0).
Would the work done on 4 normally be back-ported to 4.0? Given that each branch needs a different implementation (from my very quick look anyway), I'm guessing not.
Any chance of this being merged into 4.0 for a 4.6 release, and then cherry-picked to 4 (and fixed/improvements made) by someone on your end? We may need to stick to a fork for now if not.

Cheers.

ScopeyNZ · 2020-01-10T00:59:12Z

Would the work done on 4 normally be back-ported to 4.0? Given that each branch needs a different implementation (from my very quick look anyway), I'm guessing not.

Not unless it's a bugfix

Any chance of this being merged into 4.0 for a 4.6 release, and then cherry-picked to 4 (and fixed/improvements made) by someone on your end? We may need to stick to a fork for now if not.

Unfortunately, we can't put features like this in bugfix releases (4.0.next) as that would be breaking our semantic versioning commitments. You will have to stay on a fork, or use some form of composer-patches

I hope that doesn't discourage contributing back though, because once it's merged then we have to maintain it for you 😅

JoshuaCarter · 2020-01-10T03:12:45Z

@ScopeyNZ Thanks for the info. We'll stick to a fork for now and aim to re-target this to 4 in the near future with an eye to upgrading the module on our end, etc.

Cheers.

JoshuaCarter · 2020-03-03T01:29:48Z

@ScopeyNZ Have re-targeted this PR to 4 and tested locally. Don't think that triggers the CI tests on here so might want to manually redo those.

ScopeyNZ · 2020-03-03T02:41:00Z

As long as you switch the base branch first, it should run the suite when you force-push.

JoshuaCarter · 2020-03-03T02:57:26Z

@ScopeyNZ You can see in the timeline that I switched branches and did a force push but that didn't seem to trigger the tests.

ScopeyNZ · 2020-03-03T02:59:10Z

Looks like they did to me.

ScopeyNZ · 2020-03-03T03:00:20Z

Although it looks like the commit reference is different 🤔 . The branch and time match up though.

JoshuaCarter · 2020-03-03T03:01:09Z

Huh... I see:

Nvm I guess lol.

- Duplicates element. - Adds ' copy' or ' copy {num}' to the Title. - Reorder duplicate to position after the cloned element.

JoshuaCarter · 2020-03-03T05:52:08Z

@ScopeyNZ force pushed the top commit (no changes) to trigger. Displayed properly for me this time, so good to go afaik!

ScopeyNZ · 2020-03-03T06:31:33Z

Sure. I don't have a capability/bandwidth to give this a final once-over right now - so I'll see if I can call some favours with the SSLtd. team this week.

JoshuaCarter · 2020-03-03T07:36:12Z

Many thanks!

JoshuaCarter · 2020-03-17T23:08:06Z

Hey @ScopeyNZ Just checking for movement here. Cheers.

sachajudd

Hey @JoshuaCarter, thanks for taking the time to make a contribution back to the module. I've just had a look at what you've built compared with our initial designs and from a UX perspective I'm happy to approve this PR.

Depending on if this PR gets a review and test from a technical perspective and is merged, I'll create some issues and update our designs based off our initial thoughts now that we can see it work in practice.

Notes for @silverstripeux team for after this is merged:
Provide some direction/designs for the following scenarios:

"Unsaved changes - Opened blocks will be saved before duplicate modal is copied".
If you copy 'Content block copy 2' it becomes 'Content block copy 3', if you duplicate 'Content block copy 2' again it creates another 'Content block copy 3'. We'll look into some other options around naming conventions and what makes the most sense in this scenario.

Other:

Toast confirm message missing from the top right
Core Elemental (Impact High, UX issue) — "Copies will be set in a Draft state". After a block has been duplicated it's state indicator is 'Modified' rather than 'Draft'. I can see the colours on the class are the wrong way around. I'll look into creating a PR for this next week.

ScopeyNZ

I'm happy from a technical perspective. Note that you don't need to bundle translations yourself, as these should be overwritten on release with values from Transifex. If you've done any manual translation, you'll need to update Transifex instead.

Thanks @JoshuaCarter

ScopeyNZ · 2020-03-19T23:12:16Z

Oh, I've lost my merge ability. Lol.

sachajudd · 2020-03-19T23:12:34Z

I've just chatted with the team at Silverstripe Ltd and it looks like they'll prioritise this issue in the next month or so unless a core contributor or an owner of this module gets to it first 🙂

Note for the team: It would be great if someone could test how this works with versioned-snapshots and check to see when duplicating a block if it adds a new block in History.

Cheddam

The Duplicate operation does not trigger a snapshot in versioned-snapshots. When a block is next created, the duplicated block(s) will be included in the snapshot triggered by that operation. versioned-snapshots is handling the Elemental integration itself, so we can raise a separate issue there to get this working correctly once it's merged.

There are also some UX improvements to tackle, but they're minor and we'll split those out into separate issues to tackle at a later date.

Overall, I'm happy to merge this! Thanks so much for your contribution @JoshuaCarter, and if you have any feedback about the development tooling involved in this change we'd love to hear it.

tractorcow · 2020-12-07T00:38:00Z

I just noticed that blocks with private static $inline_editable = false; don't have a duplicate menu. We can add this in via the gridfield actions right?

srehman130 · 2021-03-03T00:53:45Z

I just noticed that blocks with private static $inline_editable = false; don't have a duplicate menu. We can add this in via the gridfield actions right?

We are having similar issues. Has anyone been able to look into this?

JoshuaCarter force-pushed the add-block-duping branch from 78f33fe to de7d25f Compare January 9, 2020 06:21

JoshuaCarter force-pushed the add-block-duping branch from de7d25f to d4bc04c Compare January 9, 2020 22:26

brynwhyman mentioned this pull request Jan 9, 2020

Duplicate Blocks #562

Closed

JoshuaCarter force-pushed the add-block-duping branch from d4bc04c to fb0689d Compare January 10, 2020 00:30

JoshuaCarter force-pushed the add-block-duping branch from fb0689d to 69c176c Compare March 2, 2020 01:28

JoshuaCarter changed the base branch from 4.0 to 4 March 3, 2020 01:21

JoshuaCarter force-pushed the add-block-duping branch from 6bcb96f to 69c176c Compare March 3, 2020 01:28

NEW adds 'Duplicate' action to elements

8617d61

- Duplicates element. - Adds ' copy' or ' copy {num}' to the Title. - Reorder duplicate to position after the cloned element.

JoshuaCarter force-pushed the add-block-duping branch from 69c176c to 8617d61 Compare March 3, 2020 05:27

sachajudd approved these changes Mar 19, 2020

View reviewed changes

ScopeyNZ approved these changes Mar 19, 2020

View reviewed changes

This was referenced Mar 20, 2020

FIX Switch incorrect modified and draft state indicator colours silverstripe/silverstripe-admin#1021

Merged

FIX Rebuild bundle.css to pull in correct state indicators #784

Merged

Cheddam self-assigned this Apr 30, 2020

Cheddam approved these changes May 1, 2020

View reviewed changes

Cheddam merged commit f378bd8 into silverstripe:4 May 1, 2020

Cheddam mentioned this pull request May 1, 2020

Support new Duplicate operation in Elemental silverstripe/silverstripe-versioned-snapshots#32

Open

clarkepaul mentioned this pull request Mar 7, 2021

Provide duplicate option for non-inline blocks #881

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add 'Duplicate' action to elements. #770

Add 'Duplicate' action to elements. #770

JoshuaCarter commented Jan 9, 2020

ScopeyNZ commented Jan 9, 2020

brynwhyman commented Jan 9, 2020

JoshuaCarter commented Jan 10, 2020

ScopeyNZ commented Jan 10, 2020

JoshuaCarter commented Jan 10, 2020

JoshuaCarter commented Mar 3, 2020

ScopeyNZ commented Mar 3, 2020

JoshuaCarter commented Mar 3, 2020

ScopeyNZ commented Mar 3, 2020

ScopeyNZ commented Mar 3, 2020

JoshuaCarter commented Mar 3, 2020

JoshuaCarter commented Mar 3, 2020

ScopeyNZ commented Mar 3, 2020 •

edited

Loading

JoshuaCarter commented Mar 3, 2020

JoshuaCarter commented Mar 17, 2020

sachajudd left a comment •

edited

Loading

ScopeyNZ left a comment

ScopeyNZ commented Mar 19, 2020

sachajudd commented Mar 19, 2020 •

edited

Loading

Cheddam left a comment

tractorcow commented Dec 7, 2020

srehman130 commented Mar 3, 2021

Add 'Duplicate' action to elements. #770

Add 'Duplicate' action to elements. #770

Conversation

JoshuaCarter commented Jan 9, 2020

ScopeyNZ commented Jan 9, 2020

brynwhyman commented Jan 9, 2020

JoshuaCarter commented Jan 10, 2020

ScopeyNZ commented Jan 10, 2020

JoshuaCarter commented Jan 10, 2020

JoshuaCarter commented Mar 3, 2020

ScopeyNZ commented Mar 3, 2020

JoshuaCarter commented Mar 3, 2020

ScopeyNZ commented Mar 3, 2020

ScopeyNZ commented Mar 3, 2020

JoshuaCarter commented Mar 3, 2020

JoshuaCarter commented Mar 3, 2020

ScopeyNZ commented Mar 3, 2020 • edited Loading

JoshuaCarter commented Mar 3, 2020

JoshuaCarter commented Mar 17, 2020

sachajudd left a comment • edited Loading

Choose a reason for hiding this comment

ScopeyNZ left a comment

Choose a reason for hiding this comment

ScopeyNZ commented Mar 19, 2020

sachajudd commented Mar 19, 2020 • edited Loading

Cheddam left a comment

Choose a reason for hiding this comment

tractorcow commented Dec 7, 2020

srehman130 commented Mar 3, 2021

ScopeyNZ commented Mar 3, 2020 •

edited

Loading

sachajudd left a comment •

edited

Loading

sachajudd commented Mar 19, 2020 •

edited

Loading