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

Add parent page to page attributes #4175

Merged
merged 2 commits into from Jan 2, 2018

Conversation

Projects
None yet
3 participants
@jorgefilipecosta
Member

jorgefilipecosta commented Dec 26, 2017

Description

This PR adds the ability to set the parent of a post/page. It also applies some refactors to the way page attributes are implemented.

How Has This Been Tested?

Add a page see that it is possible to set a parent page, set a parent page save see everything works as expected.
See that for normal posts page attributes UI does not appear.
Try to set the parent post of a post type different from pages that also supports hierarchies (plugin
"Custom Post Type UI" makes easier to do this types of tests).

Screenshots (jpeg or gifs if applicable):

image
image

Show outdated Hide outdated editor/components/page-attributes/check.js Outdated
Show outdated Hide outdated editor/components/page-attributes/order.js Outdated
Show outdated Hide outdated editor/components/page-attributes/parent.js Outdated
Show outdated Hide outdated editor/edit-post/sidebar/page-attributes/index.js Outdated
@jorgefilipecosta

This comment has been minimized.

Show comment
Hide comment
@jorgefilipecosta

jorgefilipecosta Dec 27, 2017

Member

Hi @youknowriad the reason I changed PostTypeSupportSupportCheck was to not have duplicate code requesting postType, but the concerns you raised are valid. So I end up using the same logic as before. This PR just adds the parent page field, and change PageAttributesForm to be PageAttributesOrder because that component is just the order field.

Member

jorgefilipecosta commented Dec 27, 2017

Hi @youknowriad the reason I changed PostTypeSupportSupportCheck was to not have duplicate code requesting postType, but the concerns you raised are valid. So I end up using the same logic as before. This PR just adds the parent page field, and change PageAttributesForm to be PageAttributesOrder because that component is just the order field.

@youknowriad

This comment has been minimized.

Show comment
Hide comment
@youknowriad

youknowriad Dec 27, 2017

Contributor

For some reason I don't understand :) This is breaking some styles. See the padding around a regular text block here

screen shot 2017-12-27 at 13 39 12

Contributor

youknowriad commented Dec 27, 2017

For some reason I don't understand :) This is breaking some styles. See the padding around a regular text block here

screen shot 2017-12-27 at 13 39 12

@jorgefilipecosta

This comment has been minimized.

Show comment
Hide comment
@jorgefilipecosta

jorgefilipecosta Dec 29, 2017

Member

Hi @youknowriad, changing the import to import { TermTreeSelect } from '@wordpress/blocks' solved the styles problem and it looks like things are ok now. This solution is not ideal, but I think we can use before we do the refactor that moves some component from '@wordpress/blocks' to ' @wordpress/components'.

Member

jorgefilipecosta commented Dec 29, 2017

Hi @youknowriad, changing the import to import { TermTreeSelect } from '@wordpress/blocks' solved the styles problem and it looks like things are ok now. This solution is not ideal, but I think we can use before we do the refactor that moves some component from '@wordpress/blocks' to ' @wordpress/components'.

@youknowriad

Left some minor comments, but I'm ok getting this in and moving the inspector controls separately.

jorgefilipecosta added some commits Dec 27, 2017

Renamed PageAttributesForm to PageAttributesOrder.
The component was not a form but an order field.
Add parent page to page attributes.
Adds the ability to set the parent of a post/page.

@jorgefilipecosta jorgefilipecosta merged commit e2fb127 into master Jan 2, 2018

3 checks passed

codecov/project 39.11% (-0.06%) compared to d931e5f
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@jorgefilipecosta jorgefilipecosta deleted the add/parent-page branch Jan 2, 2018

} );
const applyWithAPIDataItems = withAPIData( ( props ) => {
const restBase = get( props, 'postType.data.rest_base' );

This comment has been minimized.

@aduth

aduth Jan 2, 2018

Member

withAPIData passes a second object argument to the mapping function which includes a type function to retrieve the rest_base given a post type:

https://github.com/WordPress/gutenberg/tree/master/components/higher-order/with-api-data#usage

@aduth

aduth Jan 2, 2018

Member

withAPIData passes a second object argument to the mapping function which includes a type function to retrieve the rest_base given a post type:

https://github.com/WordPress/gutenberg/tree/master/components/higher-order/with-api-data#usage

return null;
}
const pagesTree = buildTermsTree( pageItems.filter( item => item.id !== postId ).map( ( item ) => ( {

This comment has been minimized.

@aduth

aduth Jan 2, 2018

Member

FYI: GET /wp/v2/pages has an exclude argument which could be used to exclude the current post ID instead of filtering it within the client. The big downside is that we'd lose the ability to cache/reuse between any other withAPIData requests for pages.

https://developer.wordpress.org/rest-api/reference/pages/#list-pages

@aduth

aduth Jan 2, 2018

Member

FYI: GET /wp/v2/pages has an exclude argument which could be used to exclude the current post ID instead of filtering it within the client. The big downside is that we'd lose the ability to cache/reuse between any other withAPIData requests for pages.

https://developer.wordpress.org/rest-api/reference/pages/#list-pages

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