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

Editor: Make the AddTerm Component more reusable #8588

Merged
merged 2 commits into from
Oct 11, 2016

Conversation

youknowriad
Copy link
Contributor

@youknowriad youknowriad commented Oct 7, 2016

Previously the AddTerm Component was tied with the Post. In this PR, I make it independent. Its new role is just to add a new the term.

I also provide an onSuccess callback to potentially add the created term to the edited post (It can be used for anything else).

I had also to update to AddTermForPost Action Creator to handle only the post edition and attach the term to the edited post. So, we don't need the post middleware anymore since the two actions are now independent.

I also added the possibility to enter a description for the created term (will be used for the Term Management Page).

A next step about this issue #8584 is to handle editing terms on the same component.

Testing instructions

  • Create a post
  • Add a new category to this post (using the add term modal)
  • Make sure the category is attached to the post

cc @timmyc

 - Make it independent from the post
 - Add the possibility to edit a description on the component

refs #8584
@matticbot
Copy link
Contributor

@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Oct 7, 2016
@youknowriad youknowriad added [Feature] Post/Page Editor The editor for editing posts and pages. [Status] In Progress [Type] Enhancement and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Oct 7, 2016
@youknowriad youknowriad self-assigned this Oct 7, 2016
Copy link
Contributor

@timmyc timmyc left a comment

Choose a reason for hiding this comment

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

I tested this out and it is working well. Honestly I like the move away from the middleware solution as it was kind of confusing.

One other item we need to address in making this form friendly to all Taxonomies is to toggle the dispaly of the "Top Level" checkbox, and parent selector depending on if the Taxonomy is hierarchical or not. This can be determined via the call to getPostTypeTaxonomy which returns the taxonomy object:

new_post_ bendoutdoors _wordpress_com

<FormLegend>
{ translate( 'Description', { context: 'Terms: Term description label' } ) }
</FormLegend>
<FormTextarea
Copy link
Contributor

Choose a reason for hiding this comment

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

In wp-admin the slug field is also exposed and editable. I'm thinking we should follow suit here, maybe rename showDescriptionInput to showExtendedInputs or something and have that conditionally show both description and slug?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not able to edit the slug in wp-admin. I can see it on the Categories/Tags pages but I'm not able to change it directly. but yes we can change the boolean to showExtendedInputs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the component to handle non-hierarchical taxonomies. I think we can consider merging for now. and we will handle editing once we have the listing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah sorry about that - the slug is editable in core wp-admin, but it appears we have disabled that field on wpcom. Because of that, lets follow suit here as well and do not show the slug field.

We hide the parent selector on non hierarchical taxonomies
@youknowriad youknowriad added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] In Progress labels Oct 10, 2016
@timmyc
Copy link
Contributor

timmyc commented Oct 10, 2016

@youknowriad just tested this out again on a JP site and WPCOM sites and think it is all working well. Lets get this merged in.

@timmyc timmyc added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Oct 10, 2016
@youknowriad youknowriad merged commit 7d6f7a0 into master Oct 11, 2016
@youknowriad youknowriad deleted the update/reusable-add-terms branch October 11, 2016 09:04
youknowriad added a commit that referenced this pull request Oct 11, 2016
A user noticed "Can not read hierarchical of null" error when loading the editor

This reverts commit 7d6f7a0.
youknowriad added a commit that referenced this pull request Oct 11, 2016
)

A user noticed "Can not read hierarchical of null" error when loading the editor

This reverts commit 7d6f7a0.
youknowriad added a commit that referenced this pull request Oct 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Post/Page Editor The editor for editing posts and pages. [Type] Enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants