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

[Navigation] Makes URL dynamic for pages and posts #21050

Closed
wants to merge 23 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
7223aee
adds simple dynamic link generation to navigation items
draganescu Mar 20, 2020
6610e9f
Linting and snapshots fix
draganescu Mar 23, 2020
57c9cf1
removes checking by type
draganescu Apr 27, 2020
0a79c60
adds snapshot fix
draganescu Apr 30, 2020
a0db3c8
removed URL as id from snapshot
draganescu Apr 30, 2020
a53a729
fixes snapshot
draganescu Apr 30, 2020
c1cc9cb
refactores according to code review
draganescu May 4, 2020
de76e90
removes URL when creating from existing pages
draganescu May 5, 2020
6487870
corrects docblock
draganescu May 5, 2020
7c796b7
adding stored URL back
draganescu May 7, 2020
b58ebb6
revert snapshot for adding the URL back to navigation link
draganescu May 7, 2020
6bfa53a
fixes typoin snapshot
draganescu May 11, 2020
072dfde
updates LinkControl to work with only id instead of URL
draganescu Jun 29, 2020
67fa3af
removes link from predefined navigation options, fixes create from me…
draganescu Jun 29, 2020
6373217
fixes tests
draganescu Jun 29, 2020
4362429
snapshot update
draganescu Jun 29, 2020
a23b549
fix for snapshot
draganescu Jun 30, 2020
bdcaa77
revert for rebase
draganescu Jul 9, 2020
9fded8a
updates for the new self rendering link component
draganescu Jul 9, 2020
8cbb49e
fixed for get_navigation_link_url function rename
draganescu Jul 9, 2020
f474b7f
removes id from LinkControl
draganescu Jul 9, 2020
6207832
refactors un-needed function wrapper
draganescu Jul 9, 2020
8a42dc9
Update packages/block-library/src/navigation-link/index.php
draganescu Jul 14, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 26 additions & 15 deletions packages/block-editor/src/components/link-control/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ function LinkControl( {
const [ isEditingLink, setIsEditingLink ] = useState(
forceIsEditingLink !== undefined
? forceIsEditingLink
: ! value || ! value.url
: ! value || ( ! value.url && ! value.id )
);
const [ isResolvingLink, setIsResolvingLink ] = useState( false );
const [ errorMessage, setErrorMessage ] = useState( null );
Expand Down Expand Up @@ -273,7 +273,6 @@ function LinkControl( {

return Promise.resolve( [
{
id: val,
title: val,
url: type === 'URL' ? prependHTTP( val ) : val,
type,
Expand Down Expand Up @@ -592,19 +591,31 @@ function LinkControl( {
}
) }
>
<span className="block-editor-link-control__search-item-header">
<ExternalLink
className="block-editor-link-control__search-item-title"
href={ value.url }
>
{ ( value && value.title ) || displayURL }
</ExternalLink>
{ value && value.title && (
<span className="block-editor-link-control__search-item-info">
{ displayURL }
</span>
) }
</span>
{ typeof value.url !== 'undefined' && (
<span className="block-editor-link-control__search-item-header">
<ExternalLink
className="block-editor-link-control__search-item-title"
href={ value.url }
>
{ ( value && value.title ) || displayURL }
</ExternalLink>
{ value && value.title && (
<span className="block-editor-link-control__search-item-info">
{ displayURL }
</span>
) }
</span>
) }

{ typeof value.id !== 'undefined' && (
<span className="block-editor-link-control__search-item-header">
{ value && value.title && (
<span className="block-editor-link-control__search-item-info">
{ value.title }
</span>
) }
</span>
) }
Comment on lines +610 to +618
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this part probably needs some design influence, would be good to address as a follow-up.
Screenshot 2020-07-15 at 6 01 04 pm


<Button
isSecondary
Expand Down
3 changes: 0 additions & 3 deletions packages/block-library/src/navigation-link/block.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,6 @@
"type": "boolean",
"default": false
},
"type": {
"type": "string"
},
"description": {
"type": "string"
},
Expand Down
10 changes: 6 additions & 4 deletions packages/block-library/src/navigation-link/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,12 @@ function NavigationLinkEdit( {
mergeBlocks,
onReplace,
} ) {
const { label, opensInNewTab, url, nofollow, description } = attributes;
const { label, opensInNewTab, url, id, nofollow, description } = attributes;
const link = {
url,
id,
opensInNewTab,
title: label,
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, these two aren't really the same thing. If I edit the label it doesn't mean that my page title is changed.

Not sure what the alternative is, since we don't actually know the page title.

};
const [ isLinkOpen, setIsLinkOpen ] = useState( false );
const itemLabelPlaceholder = __( 'Add link…' );
Expand Down Expand Up @@ -258,10 +260,10 @@ function NavigationLinkEdit( {
title: newTitle = '',
url: newURL = '',
opensInNewTab: newOpensInNewTab,
id,
id: newId,
} = {} ) =>
setAttributes( {
url: encodeURI( newURL ),
url: newId ? undefined : newURL,
label: ( () => {
const normalizedTitle = newTitle.replace(
/http(s?):\/\//gi,
Expand All @@ -285,7 +287,7 @@ function NavigationLinkEdit( {
return escape( normalizedURL );
} )(),
opensInNewTab: newOpensInNewTab,
id,
id: newId ? Number( newId ) : undefined,
} )
}
/>
Expand Down
35 changes: 29 additions & 6 deletions packages/block-library/src/navigation-link/index.php
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,22 @@ function block_core_navigation_link_render_submenu_icon() {
return '<svg xmlns="http://www.w3.org/2000/svg" width="12" height="12" viewBox="0 0 24 24" transform="rotate(90)"><path d="M8 5v14l11-7z"/><path d="M0 0h24v24H0z" fill="none"/></svg>';
}

/**
* Returns the link for the one nav iteem or false
*
* @param array $attrs The Navigation Link block attributes.
*
* @return string|bool Returns the determined link or false.
*/
function block_core_navigation_link_get_url( $attrs ) {
if ( isset( $attrs['id'] ) ) {
return esc_url( get_permalink( $attrs['id'] ) );
} elseif ( isset( $attrs['url'] ) ) {
return esc_url( $attrs['url'] );
}
return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

false isn't a valid URL. Maybe just omit this?

Suggested change
return false;

}

/**
* Renders the `core/navigation-link` block.
*
Expand Down Expand Up @@ -128,16 +144,23 @@ function render_block_core_navigation_link( $attributes, $content, $block ) {
$css_classes .= ' ' . $class_name;
};

$item_url = block_core_navigation_link_get_url( $attributes );
$item_tag = $item_url ? 'a' : 'span';

$html = '<li class="' . esc_attr( $css_classes . ( $has_submenu ? ' has-child' : '' ) ) .
( $is_active ? ' current-menu-item' : '' ) . '"' . $style_attribute . '>' .
'<a class="wp-block-navigation-link__content"';
'<' . $item_tag . ' class="wp-block-navigation-link__content"';

// Start appending HTML attributes to anchor tag.
if ( isset( $attributes['url'] ) ) {
$html .= ' href="' . esc_url( $attributes['url'] ) . '"';
if ( $item_url ) {
$html .= ' href="' . esc_url( $item_url ) . '"';
}

if ( isset( $attributes['opensInNewTab'] ) && true === $attributes['opensInNewTab'] ) {
if (
$item_url &&
isset( $attributes['opensInNewTab'] ) &&
true === $attributes['opensInNewTab']
) {
$html .= ' target="_blank" ';
}
// End appending HTML attributes to anchor tag.
Expand Down Expand Up @@ -171,10 +194,10 @@ function render_block_core_navigation_link( $attributes, $content, $block ) {

$html .= '</span>';

$html .= '</a>';
$html .= '</' . $item_tag . '>';
// End anchor tag content.

if ( $block->context['showSubmenuIcon'] && $has_submenu ) {
if ( isset( $block->context['showSubmenuIcon'] ) && $block->context['showSubmenuIcon'] && $has_submenu ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This change seems unrelated. Is it needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was a suggestion from @adamziel to make sure we don't get PHP Notices sometimes.

// The submenu icon can be hidden by a CSS rule on the Navigation Block.
$html .= '<span class="wp-block-navigation-link__submenu-icon">' . block_core_navigation_link_render_submenu_icon() . '</span>';
}
Expand Down
6 changes: 2 additions & 4 deletions packages/block-library/src/navigation/placeholder.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ function getSelectedMenu( selectedCreateOption ) {
* @return {WPBlock[]} An array of blocks.
*/
function mapMenuItemsToBlocks( nodes ) {
return nodes.map( ( { title, type, link: url, id, children } ) => {
return nodes.map( ( { title, type, children, object_id: id } ) => {
const innerBlocks =
children && children.length ? mapMenuItemsToBlocks( children ) : [];

Expand All @@ -92,7 +92,6 @@ function mapMenuItemsToBlocks( nodes ) {
{
type,
id,
url,
label: ! title.rendered
? __( '(no title)' )
: escape( title.rendered ),
Expand Down Expand Up @@ -131,11 +130,10 @@ function convertPagesToBlocks( pages ) {
return null;
}

return pages.map( ( { title, type, link: url, id } ) =>
return pages.map( ( { title, type, id } ) =>
createBlock( 'core/navigation-link', {
type,
id,
url,
label: ! title.rendered
? __( '(no title)' )
: escape( title.rendered ),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,36 +2,36 @@

exports[`Navigation Creating from existing Menus allows a navigation block to be created from existing menus 1`] = `
"<!-- wp:navigation {\\"orientation\\":\\"horizontal\\"} -->
<!-- wp:navigation-link {\\"label\\":\\"Home\\",\\"type\\":\\"custom\\",\\"id\\":94} /-->
<!-- wp:navigation-link {\\"label\\":\\"Home\\",\\"id\\":94} /-->

<!-- wp:navigation-link {\\"label\\":\\"Accusamus quo repellat illum magnam quas\\",\\"type\\":\\"post_type\\",\\"id\\":95} -->
<!-- wp:navigation-link {\\"label\\":\\"Debitis cum consequatur sit doloremque\\",\\"type\\":\\"post_type\\",\\"id\\":96} /-->
<!-- wp:navigation-link {\\"label\\":\\"Accusamus quo repellat illum magnam quas\\",\\"id\\":41} -->
<!-- wp:navigation-link {\\"label\\":\\"Debitis cum consequatur sit doloremque\\",\\"id\\":51} /-->
<!-- /wp:navigation-link -->

<!-- wp:navigation-link {\\"label\\":\\"Est ea vero non nihil officiis in\\",\\"type\\":\\"post_type\\",\\"id\\":97} -->
<!-- wp:navigation-link {\\"label\\":\\"Fuga odio quis tempora\\",\\"type\\":\\"post_type\\",\\"id\\":98} -->
<!-- wp:navigation-link {\\"label\\":\\"In consectetur repellendus eveniet maiores aperiam\\",\\"type\\":\\"post_type\\",\\"id\\":99} -->
<!-- wp:navigation-link {\\"label\\":\\"Mollitia maiores consequatur ea dolorem blanditiis\\",\\"type\\":\\"post_type\\",\\"id\\":100} -->
<!-- wp:navigation-link {\\"label\\":\\"Necessitatibus nisi qui qui necessitatibus quaerat possimus\\",\\"type\\":\\"post_type\\",\\"id\\":101} /-->
<!-- wp:navigation-link {\\"label\\":\\"Est ea vero non nihil officiis in\\",\\"id\\":53} -->
<!-- wp:navigation-link {\\"label\\":\\"Fuga odio quis tempora\\",\\"id\\":56} -->
<!-- wp:navigation-link {\\"label\\":\\"In consectetur repellendus eveniet maiores aperiam\\",\\"id\\":15} -->
<!-- wp:navigation-link {\\"label\\":\\"Mollitia maiores consequatur ea dolorem blanditiis\\",\\"id\\":45} -->
<!-- wp:navigation-link {\\"label\\":\\"Necessitatibus nisi qui qui necessitatibus quaerat possimus\\",\\"id\\":27} /-->
<!-- /wp:navigation-link -->
<!-- /wp:navigation-link -->
<!-- /wp:navigation-link -->
<!-- /wp:navigation-link -->

<!-- wp:navigation-link {\\"label\\":\\"Nulla omnis autem dolores eligendi\\",\\"type\\":\\"post_type\\",\\"id\\":102} /-->
<!-- wp:navigation-link {\\"label\\":\\"Nulla omnis autem dolores eligendi\\",\\"id\\":43} /-->

<!-- wp:navigation-link {\\"label\\":\\"Sample Page\\",\\"type\\":\\"post_type\\",\\"id\\":103} /-->
<!-- wp:navigation-link {\\"label\\":\\"Sample Page\\",\\"id\\":2} /-->

<!-- wp:navigation-link {\\"label\\":\\"Beatae qui labore voluptas eveniet officia quia voluptas qui porro sequi et aut est\\",\\"type\\":\\"taxonomy\\",\\"id\\":104} -->
<!-- wp:navigation-link {\\"label\\":\\"Et minus itaque velit tempore hic quisquam saepe quas asperiores\\",\\"type\\":\\"taxonomy\\",\\"id\\":105} -->
<!-- wp:navigation-link {\\"label\\":\\"Et quas a et mollitia et voluptas optio voluptate quia quo unde aut in nostrum iste impedit quisquam id aut\\",\\"type\\":\\"taxonomy\\",\\"id\\":106} -->
<!-- wp:navigation-link {\\"label\\":\\"Illo quis sit impedit itaque expedita earum deserunt magni doloremque velit eum id error\\",\\"type\\":\\"taxonomy\\",\\"id\\":107} /-->
<!-- wp:navigation-link {\\"label\\":\\"Beatae qui labore voluptas eveniet officia quia voluptas qui porro sequi et aut est\\",\\"id\\":7} -->
<!-- wp:navigation-link {\\"label\\":\\"Et minus itaque velit tempore hic quisquam saepe quas asperiores\\",\\"id\\":19} -->
<!-- wp:navigation-link {\\"label\\":\\"Et quas a et mollitia et voluptas optio voluptate quia quo unde aut in nostrum iste impedit quisquam id aut\\",\\"id\\":6} -->
<!-- wp:navigation-link {\\"label\\":\\"Illo quis sit impedit itaque expedita earum deserunt magni doloremque velit eum id error\\",\\"id\\":16} /-->
<!-- /wp:navigation-link -->
<!-- /wp:navigation-link -->
<!-- /wp:navigation-link -->

<!-- wp:navigation-link {\\"label\\":\\"WordPress.org\\",\\"type\\":\\"custom\\",\\"id\\":108} -->
<!-- wp:navigation-link {\\"label\\":\\"Google\\",\\"type\\":\\"custom\\",\\"id\\":109} /-->
<!-- wp:navigation-link {\\"label\\":\\"WordPress.org\\",\\"id\\":108} -->
<!-- wp:navigation-link {\\"label\\":\\"Google\\",\\"id\\":109} /-->
<!-- /wp:navigation-link -->
<!-- /wp:navigation -->"
`;
Expand All @@ -46,19 +46,19 @@ exports[`Navigation Creating from existing Menus does not display option to crea

exports[`Navigation Creating from existing Pages allows a navigation block to be created using existing pages 1`] = `
"<!-- wp:navigation {\\"orientation\\":\\"horizontal\\"} -->
<!-- wp:navigation-link {\\"label\\":\\"Home\\",\\"type\\":\\"page\\",\\"id\\":1,\\"url\\":\\"https://this/is/a/test/page/home\\"} /-->
<!-- wp:navigation-link {\\"label\\":\\"Home\\",\\"id\\":1} /-->

<!-- wp:navigation-link {\\"label\\":\\"About\\",\\"type\\":\\"page\\",\\"id\\":2,\\"url\\":\\"https://this/is/a/test/page/about\\"} /-->
<!-- wp:navigation-link {\\"label\\":\\"About\\",\\"id\\":2} /-->

<!-- wp:navigation-link {\\"label\\":\\"Contact Us\\",\\"type\\":\\"page\\",\\"id\\":3,\\"url\\":\\"https://this/is/a/test/page/contact\\"} /-->
<!-- wp:navigation-link {\\"label\\":\\"Contact Us\\",\\"id\\":3} /-->
<!-- /wp:navigation -->"
`;

exports[`Navigation Creating from existing Pages does not display option to create from existing Pages if there are no Pages 1`] = `"<!-- wp:navigation {\\"orientation\\":\\"horizontal\\"} /-->"`;

exports[`Navigation allows an empty navigation block to be created and manually populated using a mixture of internal and external links 1`] = `
"<!-- wp:navigation {\\"orientation\\":\\"horizontal\\"} -->
<!-- wp:navigation-link {\\"label\\":\\"WP\\",\\"id\\":\\"https://wordpress.org\\",\\"url\\":\\"https://wordpress.org\\"} /-->
<!-- wp:navigation-link {\\"label\\":\\"WP\\",\\"url\\":\\"https://wordpress.org\\"} /-->

<!-- wp:navigation-link {\\"label\\":\\"Contact\\",\\"id\\":1,\\"url\\":\\"https://this/is/a/test/search/get-in-touch\\"} /-->
<!-- /wp:navigation -->"
Expand Down