Skip to content
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

FIX Adding some page updates after sorting: #553

Conversation

ScopeyNZ
Copy link
Contributor

  • The status of the block (switching to unpublished/draft) now correctly changes
  • An update of the preview is triggered
  • Fixes an issue where required props were not being provided to ElementActions in a test

Fixes #547

- The status of the block (switching to unpublished/draft) now correctly changes
- An update of the preview is triggered
- Fixes an issue where required props were not being provided to ElementActions in a test

Fixes silverstripe#547
Copy link
Contributor

@NightJar NightJar left a comment

Choose a reason for hiding this comment

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

Nice commit message, that gave some important context I was wondering about :D

@NightJar NightJar merged commit d99cb91 into silverstripe:4.0 Dec 18, 2018
@NightJar NightJar deleted the pulls/4.0/getting-this-sort-together branch December 18, 2018 01:41
@@ -57,7 +59,10 @@ class ElementEditor extends PureComponent {
handleDragEnd(sourceId, afterId) {
const { actions: { handleSortBlock }, areaId } = this.props;

handleSortBlock(sourceId, afterId, areaId);
handleSortBlock(sourceId, afterId, areaId).then(() => {
const preview = window.jQuery('.cms-preview');
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a thought - this doesn't have any handling for where this selector doesn't match. I suppose that means you won't be able to use Elemental outside of the CMS (e.g. in a ModelAdmin). We should be careful to handle edge cases like this I think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes that's reasonable and I agree. While I was doing this I was thinking it would be nice to consolidate a lot of this "entwine hack" code into some lib js file. Preferably available in admin or something. There's a bunch of places we use this code now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants