Clarify explanation of how 'Convert to Links' works in Page List block #45394

artemiomorales · 2022-10-28T19:37:10Z

What?

Fixes #44615.
This pull request clarifies the copy for the Convert to Links functionality in the Page List block, explaining where the current menu comes from and providing a link to the documentation, as per this issue.

Why?

This will make the functionality clearer to users and cut down on confusion.

How?

It's just a simple update of the copy.

Testing Instructions

Select a Page List block in Full Site Editing mode
Click the 'edit' button

Screenshot

codesandbox · 2022-10-28T19:37:12Z

Open in CodeSandbox Web Editor | VS Code | VS Code Insiders

github-actions · 2022-10-28T19:37:39Z

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @artemiomorales! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

dmsnell · 2022-10-28T23:00:46Z

packages/block-library/src/page-list/convert-to-links-modal.js

+				<a href="https://wordpress.org/support/article/navigation-block/">
+					{ __( 'Click here' ) }
+				</a>
+				{ __( ' to learn more about menu management.' ) }


we can't split and concatenate translated strings because they won't translate properly. instead we need to use the translation helper createInterpolateElement so that the entire chunk can be translated as a whole unit.

{ createInterpolateElement( __( '<a>Click here</a> to learn more about menu management.' ), { a: <a href="https://wordpress.org/support/article/navigation-block" /> ) }

dmsnell

I like the wording updates here but I don't feel confident in approving the changes; please defer to what others say.

We do need to make sure though that we aren't splitting and concatenating translated strings, so once that's resolved I'll remove my block on this.

artemiomorales · 2022-10-29T15:36:54Z

Thanks for checking this @dmsnell! The splitting of the translated string has been removed.

dmsnell · 2022-10-29T16:27:46Z

Looks better @artemiomorales - just another poke though, it was fine by me to have "Click here" be the link; the only necessity was using the right translation tools to ensure it didn't break in other languages. 👍

artemiomorales · 2022-10-29T20:45:07Z

@dmsnell Got it. Do you want the a tag text to be restricted to 'Click here' like it was previously before you approve this?

So this is my first time doing i18n, and I looked at internationalizing JavaScript in the handbook, as well as How to Internationalize Your Plugin, Internationalization and @wordpress/i18n.

I'm not sure how I'd go about doing this, but would consider using dangerouslySetInnerHTML, using translation files, or perhaps using wp_localize_script(), all of which seem like overkill when we can just set the link to the whole line, which would be clearer maintenance-wise and probably be a better UX overall.

Let me know your thoughts!

dmsnell · 2022-10-30T02:01:24Z

Do you want the a tag text to be restricted to 'Click here' like it was previously before you approve this?

I don't have any feelings on it; I simply responded because I saw you changed more than I had asked for, and I want you to be empowered to make the changes you want without running into the risks.

I'm not sure how I'd go about doing this

Did you try the code I included as an example? That should produce a new React component you can embed like any other.

const HelpLink = createInterpolateElement(…);

return <div><HelpLink /></div>;

artemiomorales · 2022-10-30T14:29:50Z

@dmsnell

Did you try #45394 (comment)? That should produce a new React component you can embed like any other.

Apologies! I somehow missed that comment. Thank you for providing this snippet; that makes everything much clearer and is good info for the future. I also have no strong preference either way and decided to just go ahead and commit your suggestion. Much appreciated 🙏

artemiomorales · 2022-10-30T14:42:01Z

Worth noting though: Using createInterpolateElement as suggested causes the linter to fail. I overrode that on my local, but it looks like the checks are also performed here on Github and failed as well, so not sure if that's an nonissue, if there's an alternative syntax that would be better to use, or if omitting createInterpolateElement would be best.

getdave

Thank you for taking this one. Very much appreciated.

I wonder if @jasmussen has a view on the wording here?

getdave · 2022-10-31T09:54:34Z

packages/block-library/src/page-list/convert-to-links-modal.js

+			</p>
+			<p>
+				{ createInterpolateElement(
+					__( '<a>Click here</a> to learn more about menu management.' ),


Let's avoid "Click here" as the link. This isn't good for a11y.

The wording should/could be:

Learn more about menu management.

And the whole string could be the link. That way if an assistive tech device encounters the link when browsing the page by hyperlinks it [the link] will make sense without additional context.

getdave · 2022-10-31T09:56:29Z

packages/block-library/src/page-list/convert-to-links-modal.js

+			<p>
+				{ createInterpolateElement(
+					__( '<a>Click here</a> to learn more about menu management.' ),
+					{ a: <a href="https://wordpress.org/support/article/navigation-block" /> }


Shall we make this open in a new tab?

draganescu

I like the updates, there is clearly an improvement over the dry text we have now, but I have a few nits in the comments.

draganescu · 2022-10-31T09:55:54Z

packages/block-library/src/page-list/convert-to-links-modal.js

 				) }
 			</p>
 			<p>
 				{ __(
-					"Note: if you add new pages to your site, you'll need to add them to your navigation menu."
+					'If you previously created a menu in a classic theme, converting will allow you to import it.'


I don't think this is correct! You can import classic menus from the menu selector in the inspector even if your current menu is a page list or contains a page list. Or am I missing something?

The experience is that you need to convert to links or create new in order to import a classic menu.

We might like to offer the import feature in the sidebar in all circumstances.

Also remember that the auto-import of classic menus is now in the Gutenberg Plugin so if you have an existing classic menu it will automatically be used and you will never see the page list.

draganescu · 2022-10-31T09:56:51Z

packages/block-library/src/page-list/convert-to-links-modal.js

+			</p>
+			<p>
+				{ createInterpolateElement(
+					__( '<a>Click here</a> to learn more about menu management.' ),


I love this addition!

I cold not agree more. Providing inline support is key.

I have dropped a comment in the docs tracker to ensure the support article is up to date. WordPress/Documentation-Issue-Tracker#297 (comment)

draganescu · 2022-10-31T09:58:53Z

packages/block-library/src/page-list/convert-to-links-modal.js

 			className={ 'wp-block-page-list-modal' }
 			aria={ { describedby: 'wp-block-page-list-modal__description' } }
 		>
 			<p id={ 'wp-block-page-list-modal__description' }>
 				{ __(
-					'To edit this navigation menu, convert it to single page links. This allows you to add, re-order, remove items, or edit their labels.'
+					'We’ve provided a default list of your published pages so you can get started quickly. If you’d prefer to edit this menu, click the Convert button below — converting will allow you to add, re-order, and remove items, or edit their labels.'


Who is "we"? Do we have other places with this form of addressing in WP?

Coule the disatvantage be squeezed in? Converting will lose the auto updating of the menu.

@draganescu Is there a "tone of voice" document somewhere for guidance on writing these sorts of user facing messages? Surely Core has something although I came up with nothing when searching.

Good points, @draganescu!

I had not thought about it but I get that the "we" can sound a bit odd. How about the following:
"This is a default list of your published pages so you can get started quickly."

I agree that we should mention how this change will prevent "auto-updating" from working. Maybe we could add it at the very end:
"Please note that if you add new pages to your site, you'll need to manually add them to your navigation menu."

jasmussen · 2022-10-31T12:10:26Z

Thank you for the ping Dave, thank you for the PR Artemio and the original issue Alvaro!

It seems fine to edit the text to provide clarity. However a few notes:

A link to documentation is perhaps acceptable, but this feels like a "one-off" location. If I want to read about menu management, I shouldn't have to click "Edit" first, and it might not be a great precedent to provide such links.
That isn't to say we can't add a link to documentation. But since it links to just the navigation block page, any context provided by the modal is immediately lost and you're back to reading from the top. Unless we can link with more precision, it seems better to find a more systematic approach to adding block documentation.
The more text in the modal, the less likely it is someone will read it, and the more scary it looks. Anything we can do to rephase it shorter, the more clarity this can add.

The main improvement we can add is clarity around where these pages are coming from. But the current first paragraph is a little confusing to me. Perhaps something along these lines?

This navigation is showing all pages on your site. If you'd like to add, remove, or reorder items, it must be converted to a custom menu.

The following phrase was removed. Are we sure we don't need it? The fact that the Page List menu stays in sync when you add or remove pages from your site feels to me like the single most important thing to highlight.

Note: if you add new pages to your site, you'll need to add them to your navigation menu.

The following is confusing to me:

If you previously created a menu in a classic theme, converting will allow you to import it.

Don't classic menus automatically get converted?
If they don't, can't we handle this automatically, without highlighting in a message like this?

CC: @SaxonF as he's had some thoughts around repurposing the Page List block to be called "Auto-menu", that term could potentially be useful in the prose here.

artemiomorales · 2022-10-31T13:42:49Z

Thank you all for the feedback and discussion!

I can update the pull request after @SaxonF has a chance to offer insight, and once there's agreement on what the wording should be and how to handle the link to the documentation.

mrfoxtalbot · 2022-11-01T07:29:46Z

Thank you for your thoughts @jasmussen

The fact that the Page List menu stays in sync when you add or remove pages from your site feels to me like the single most important thing to highlight.

I agree. We should keep this sentence. I also agree about the fact that having more text reduces the chances of people reading through and make it more scary.

But since it links to just the navigation block page, any context provided by the modal is immediately lost and you're back to reading from the top.

We could link to https://wordpress.org/support/article/navigation-block/#add-all-pages instead? (please note that the article is pending an update)

Unless we can link with more precision, it seems better to find a more systematic approach to adding block documentation.

I would love to find a systematic approach to adding unobtrusive inline support links. Are you aware of any initiatives around this?

jasmussen · 2022-11-01T09:00:12Z

We could link to https://wordpress.org/support/article/navigation-block/#add-all-pages instead? (please note that the article is pending an update)

The problem is, the "Add all pages" button doesn't exist anymore, and big efforts are underway to improve the flow in #42257.

I would love to find a systematic approach to adding unobtrusive inline support links. Are you aware of any initiatives around this?

I've seen in a few places links added to the block description, here:

However this is mainly for the most complex of blocks like query as shown here, and would not be appropriate for Paragraph, for instance.

I recognize the navigation block has a great deal of complexity and would meet that threshold. The main challenge there is that adding more stuff to the inspector at this point only makes things worse. However there's an effort underway that might fit additional help text and/or links in the inspector: #40204

Perhaps the best part at this point is to omit the link for now, update the documentation to be fresher, and then revisit adding a link to documentation separately, perhaps after 40204 lands?

SaxonF · 2022-11-02T06:39:35Z

Perhaps the best part at this point is to omit the link for now, update the documentation to be fresher, and then revisit adding a link to documentation separately, perhaps after 40204 lands?

+1

If they don't, can't we handle this automatically, without highlighting in a message like this?

+1

A suggestion on copy below but feel free to ignore. "Convert" felt a little too techie to me as the primary CTA.

artemiomorales · 2022-11-04T21:54:45Z

I updated the text with @SaxonF's suggestion.

Also, there is indeed a WordPress Brand Writing Style Guide! In particular, it recommends the use of American English, so I changed the spelling from 'Organise' to 'Organize.' (@getdave mentioned interest in a style guide like this).

Let me know how things look! Am happy to keep iterating.

getdave

Thank you for being so receptive to feedback and for continuing to iterate here.

I tested this and the wording seems fine to me with the exception of the "tip" (see comment).

getdave · 2022-11-09T01:16:29Z

packages/block-library/src/page-list/convert-to-links-modal.js

 				) }
 			</p>
 			<p>
 				{ __(
-					"Note: if you add new pages to your site, you'll need to add them to your navigation menu."
+					'Tip: If you want to return to a managed menu, simply add a new menu item and pick the Pages List block.'


As things stand there doesn't appear to be a way to achieve this. Try it out and you'll see what I mean.

Therefore I don't think we can include this "tip" until we have a new PR which enables this flow.

Once this is removed we're good to merge this.

artemiomorales · 2022-11-09T18:35:09Z

@getdave is right, thanks for testing this.

When I try to add a new Page List, the original styling is lost:

Even if I try to insert a new Header pattern and start over, I'm unable to recreate the header as it was originally; it utilizes a Navigation block instead of a Page List block:

In other words, as it stands, once a Page List block is converted inside of the default header, one is unable to return to the original implementation.

Perhaps the text should be revised to the following instead, at least until this can be improved? Or perhaps this is a bug that needs to be addressed, and we should refrain from moving forward on this issue until that is resolved.

Customize this menu

This menu is automatically kept in sync with pages on your site. You can manage the menu yourself by clicking customize below.

Warning: If you want to return to a managed menu, you can add a new Pages List block to this header, but the original styling will be lost.

artemiomorales · 2022-11-09T19:07:05Z

After additional testing, I realized you can get the original implementation back if you add a new Navigation block to the header:

Perhaps the text could be the following then:

Customize this menu

This menu is automatically kept in sync with pages on your site. You can manage the menu yourself by clicking customize below.

Tip: If you want to return to a managed menu, simply add a new Navigation block to this header.

getdave · 2022-11-09T21:16:46Z

Tip: If you want to return to a managed menu, simply add a new Navigation block to this header.

I would just remove the Tip for now. We can't hard code in a reference to a particular template (e.g. this header).

We can raise an Issue to track the need to be able to revert to a "managed menu" although I suspect it will be a less common use case.

draganescu · 2022-11-10T12:53:36Z

I agree with @getdave - because the best solution to the problem the tip found is that we should have a one button action somewhere to revert to "managed menu". In other words maybe an attribute of the navigation block that overrides how the fallback performs in the sens that if attribute is true then it always shows and renders a page list block?

SaxonF · 2022-11-10T18:05:04Z

@getdave I was originally thinking about the pages block as basically a setting at the nav level of the sub nav level (e.g. just a toggle) but the issue I ran into was how to handle search, logo etc. There needs to be a "line" between what's managed and what's not, and the pages block is that line at the moment.

One potential solution is for the navigation block to focus purely on nav related items. Logo, search etc would just exist at the header level outside the navigation block.

getdave · 2022-11-15T16:20:45Z

@artemiomorales Is there anything I can do to help you with landing this one?

…ation

artemiomorales · 2022-11-16T13:06:30Z

@getdave I don't believe so! I just updated the branch and removed the tip. Just let me know if I should make any additional changes 🙏

artemiomorales requested a review from tellthemachines as a code owner October 28, 2022 19:37

artemiomorales mentioned this pull request Oct 28, 2022

Provide a clearer explanation about how "Convert to links" works & include a link to the documentation. #44615

Closed

github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Oct 28, 2022

artemiomorales added [Type] Enhancement A suggestion for improvement. Good First Review A PR that's suitable for someone looking to contribute for the first time by reviewing code [Block] Navigation Affects the Navigation Block labels Oct 28, 2022

dmsnell reviewed Oct 28, 2022

View reviewed changes

dmsnell requested changes Oct 28, 2022

View reviewed changes

dmsnell self-requested a review October 29, 2022 16:27

getdave requested review from MaggieCabrera, scruffian, getdave, ajlende and draganescu October 31, 2022 09:52

getdave requested changes Oct 31, 2022

View reviewed changes

draganescu reviewed Oct 31, 2022

View reviewed changes

artemiomorales force-pushed the update/revise-page-list-edit-message branch from 4c2fe7f to 585e356 Compare November 4, 2022 21:50

georgeh mentioned this pull request Nov 6, 2022

WIP Off Canvas Navigation: Edit Button #45575

Closed

4 tasks

getdave reviewed Nov 9, 2022

View reviewed changes

artemiomorales added 5 commits November 16, 2022 07:56

Clarify explanation of how 'Convert to Links' works

2c7b5a2

Remove splitting of translated string

6068d2e

Update string to use createInterpolateElement to ensure proper transl…

4dee009

…ation

Simplify and clarify language in Page List edit modal

917677d

Remove tip from modal copy

fe07bac

artemiomorales force-pushed the update/revise-page-list-edit-message branch from 585e356 to fe07bac Compare November 16, 2022 13:04

getdave enabled auto-merge (squash) November 16, 2022 21:28

dmsnell approved these changes Nov 21, 2022

View reviewed changes

getdave approved these changes Nov 22, 2022

View reviewed changes

getdave merged commit 516d05c into WordPress:trunk Nov 22, 2022

github-actions bot added this to the Gutenberg 14.7 milestone Nov 22, 2022

femkreations added the Needs User Documentation Needs new user documentation label Jan 31, 2023

femkreations mentioned this pull request Feb 27, 2023

[HelpHub] Page List block WordPress/Documentation-Issue-Tracker#676

Closed

23 tasks

Clarify explanation of how 'Convert to Links' works in Page List block #45394

Clarify explanation of how 'Convert to Links' works in Page List block #45394

Conversation

artemiomorales commented Oct 28, 2022 • edited by noisysocks Loading

What?

Why?

How?

Testing Instructions

Screenshot

codesandbox bot commented Oct 28, 2022

github-actions bot commented Oct 28, 2022

Choose a reason for hiding this comment

dmsnell left a comment

Choose a reason for hiding this comment

artemiomorales commented Oct 29, 2022

dmsnell commented Oct 29, 2022

artemiomorales commented Oct 29, 2022

dmsnell commented Oct 30, 2022

artemiomorales commented Oct 30, 2022

artemiomorales commented Oct 30, 2022

getdave left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

draganescu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrfoxtalbot Oct 31, 2022 • edited Loading

Choose a reason for hiding this comment

jasmussen commented Oct 31, 2022 • edited Loading

artemiomorales commented Oct 31, 2022

mrfoxtalbot commented Nov 1, 2022

jasmussen commented Nov 1, 2022

SaxonF commented Nov 2, 2022

artemiomorales commented Nov 4, 2022

getdave left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

artemiomorales commented Nov 9, 2022

artemiomorales commented Nov 9, 2022

getdave commented Nov 9, 2022

draganescu commented Nov 10, 2022

SaxonF commented Nov 10, 2022

getdave commented Nov 15, 2022

artemiomorales commented Nov 16, 2022

artemiomorales commented Oct 28, 2022 •

edited by noisysocks

Loading

mrfoxtalbot Oct 31, 2022 •

edited

Loading

jasmussen commented Oct 31, 2022 •

edited

Loading