Skip to content

Editing entities from select2. - #12

Merged
valendesigns merged 16 commits into
developfrom
feature/edit-entities
Sep 24, 2016
Merged

valendesigns merged 16 commits into
developfrom
feature/edit-entities

Conversation

@miina

@miina miina commented Aug 31, 2016 •

Copy link
Copy Markdown
Contributor

Fixes #8

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

Copy link
Copy Markdown
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
Copy Markdown
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

miina commented Sep 1, 2016

Copy link
Copy Markdown
Contributor Author

@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
Copy Markdown
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
Copy Markdown
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

miina commented Sep 2, 2016

Copy link
Copy Markdown
Contributor Author

I think the scrolling issue is fixed.

@westonruter

Copy link
Copy Markdown
Contributor

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

var component = this;

// Set up the add new post buttons
component.container.on( 'click', '.select2-selection__choice__edit', function() {

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

return $.trim( component.select2_result_template( data ) );
},
templateSelection: function( data ) {
data.multiple = component.select2_options.multiple;

Copy link
Copy Markdown
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
Copy Markdown
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