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

Editing entities from select2. #12

Merged
merged 16 commits into from
Sep 24, 2016
Merged

Editing entities from select2. #12

merged 16 commits into from
Sep 24, 2016

Conversation

miina
Copy link
Contributor

@miina miina commented Aug 31, 2016

Fixes #8

return $.trim( component.select2_result_template( data ) );
},
templateSelection: function( data ) {
data.disable_edit = ! data.id;
Copy link
Contributor

Choose a reason for hiding this comment

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

The id is not defined when the template is for the “searching” message or for the “no results found” message?

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, and for placeholders as well.

@miina
Copy link
Contributor Author

miina commented Sep 1, 2016

@westonruter Found that data.element is present only in case the object has to be link. Removed the disable_edit and replaced with using element.

@valendesigns
Copy link
Contributor

@miina This is working exactly how I had envisioned it, but I found an obscure bug. If you drag & drop the order of the posts, then edit one and return focus back to the control the Customizer panel will not scroll up or down. I cannot reproduce the issue without reordering the posts first.

@westonruter
Copy link
Contributor

Instead of making the result text itself a link, what if there was a link added after the text that had the edit dashicon as its content (along with screen-reader-text)? This would ensure that the main text itself could still have the cursor:move whereas the link itself to edit could have cursor:pointer.

@miina
Copy link
Contributor Author

miina commented Sep 2, 2016

I think the scrolling issue is fixed.

@westonruter
Copy link
Contributor

FYI: This same functionality is also proposed for the dropdown-pages core control: xwp/wp-customize-posts#271


// Set up the add new post buttons
component.container.on( 'click', '.select2-selection__choice__edit', function() {
var ensuredPromise, returnPromise, postId, $elm = $( this );
Copy link
Contributor

Choose a reason for hiding this comment

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

The body of this function can now be replaced with a call to api.Posts.startEditPostFlow(). See https://github.com/xwp/wp-customize-posts/blob/33840418da9dcd7c55d15061de5fe49f85d9058f/js/customize-posts.js#L589-L598

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing that out, will take a look.

@westonruter
Copy link
Contributor

As part of this I suggest the style of the create and edit buttons get updated to match the dropdown-pages now in Customize Posts:

screen shot 2016-09-17 at 3 05 00 pm

It's using dashicons for the icons and using flexbox to lay-out the controls.

@@ -111,6 +111,7 @@ wp.customize.ObjectSelectorComponent = (function( api, $ ) {
return $.trim( component.select2_result_template( data ) );
},
templateSelection: function( data ) {
data.multiple = component.select2_options.multiple;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a better way how to get it from template that it's multiple select field?

Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't look like. I checked this and if there are any other arguments, and I see none. So yeah, looks good.

@valendesigns valendesigns merged commit 3a15033 into develop Sep 24, 2016
@westonruter westonruter added this to the 0.4.0 milestone Jan 9, 2017
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.

3 participants