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

RichText/Footnotes: make getRichTextValues work with InnerBlocks.Content #52241

Merged
merged 7 commits into from
Jul 10, 2023

Conversation

ellatrix
Copy link
Member

@ellatrix ellatrix commented Jul 3, 2023

What?

Fixes #52084.

getRichTextValues currently does not work with InnerBlocks.Content because this wraps useInnerBlocksProps.save in a function that is not called when the tree is constructed. This PR will now "render" all the components in the save function, meaning it will just call all the functions to get the nested element trees and check for rich text props in there. It's similar to renderToString in the @wordpress/element package, but without creating a string. We just need to find rich text values and can skip all the rest.

Why?

Footnotes is broken for blocks using InnerBlocks.Content;

How?

See above.

Testing Instructions

Create a list blocks and add a footnote. It should add an entry in the footnotes block.

Testing Instructions for Keyboard

Screenshots or screencast

@ellatrix ellatrix added [Type] Bug An existing feature does not function as intended [Block] Footnotes Affects the Footnotes Block labels Jul 3, 2023
@ellatrix ellatrix added this to the Gutenberg 16.2 milestone Jul 3, 2023
@github-actions
Copy link

github-actions bot commented Jul 3, 2023

Size Change: +2.08 kB (0%)

Total Size: 1.43 MB

Filename Size Change
build/block-editor/content-rtl.css 4.26 kB +12 B (0%)
build/block-editor/content.css 4.25 kB +12 B (0%)
build/block-editor/index.min.js 209 kB +117 B (0%)
build/block-library/blocks/details/style-rtl.css 178 B +19 B (+12%) ⚠️
build/block-library/blocks/details/style.css 178 B +19 B (+12%) ⚠️
build/block-library/blocks/query-pagination/style-rtl.css 302 B +14 B (+5%) 🔍
build/block-library/blocks/query-pagination/style.css 299 B +15 B (+5%) 🔍
build/block-library/index.min.js 201 kB +9 B (0%)
build/block-library/style-rtl.css 13.7 kB +23 B (0%)
build/block-library/style.css 13.7 kB +22 B (0%)
build/core-data/index.min.js 16.1 kB +19 B (0%)
build/edit-site/index.min.js 87.6 kB +427 B (0%)
build/edit-site/style-rtl.css 13 kB +169 B (+1%)
build/edit-site/style.css 13 kB +168 B (+1%)
build/react-refresh-entry/index.min.js 9.47 kB +1.03 kB (+12%) ⚠️
ℹ️ View Unchanged
Filename Size
build/a11y/index.min.js 955 B
build/annotations/index.min.js 2.69 kB
build/api-fetch/index.min.js 2.28 kB
build/autop/index.min.js 2.1 kB
build/blob/index.min.js 451 B
build/block-directory/index.min.js 6.99 kB
build/block-directory/style-rtl.css 1.02 kB
build/block-directory/style.css 1.02 kB
build/block-editor/default-editor-styles-rtl.css 381 B
build/block-editor/default-editor-styles.css 381 B
build/block-editor/style-rtl.css 14.8 kB
build/block-editor/style.css 14.8 kB
build/block-library/blocks/archives/editor-rtl.css 61 B
build/block-library/blocks/archives/editor.css 60 B
build/block-library/blocks/archives/style-rtl.css 90 B
build/block-library/blocks/archives/style.css 90 B
build/block-library/blocks/audio/editor-rtl.css 150 B
build/block-library/blocks/audio/editor.css 150 B
build/block-library/blocks/audio/style-rtl.css 122 B
build/block-library/blocks/audio/style.css 122 B
build/block-library/blocks/audio/theme-rtl.css 126 B
build/block-library/blocks/audio/theme.css 126 B
build/block-library/blocks/avatar/editor-rtl.css 116 B
build/block-library/blocks/avatar/editor.css 116 B
build/block-library/blocks/avatar/style-rtl.css 104 B
build/block-library/blocks/avatar/style.css 104 B
build/block-library/blocks/block/editor-rtl.css 305 B
build/block-library/blocks/block/editor.css 305 B
build/block-library/blocks/button/editor-rtl.css 584 B
build/block-library/blocks/button/editor.css 582 B
build/block-library/blocks/button/style-rtl.css 624 B
build/block-library/blocks/button/style.css 623 B
build/block-library/blocks/buttons/editor-rtl.css 337 B
build/block-library/blocks/buttons/editor.css 337 B
build/block-library/blocks/buttons/style-rtl.css 332 B
build/block-library/blocks/buttons/style.css 332 B
build/block-library/blocks/calendar/style-rtl.css 239 B
build/block-library/blocks/calendar/style.css 239 B
build/block-library/blocks/categories/editor-rtl.css 113 B
build/block-library/blocks/categories/editor.css 112 B
build/block-library/blocks/categories/style-rtl.css 124 B
build/block-library/blocks/categories/style.css 124 B
build/block-library/blocks/code/editor-rtl.css 53 B
build/block-library/blocks/code/editor.css 53 B
build/block-library/blocks/code/style-rtl.css 121 B
build/block-library/blocks/code/style.css 121 B
build/block-library/blocks/code/theme-rtl.css 124 B
build/block-library/blocks/code/theme.css 124 B
build/block-library/blocks/columns/editor-rtl.css 108 B
build/block-library/blocks/columns/editor.css 108 B
build/block-library/blocks/columns/style-rtl.css 409 B
build/block-library/blocks/columns/style.css 409 B
build/block-library/blocks/comment-author-avatar/editor-rtl.css 125 B
build/block-library/blocks/comment-author-avatar/editor.css 125 B
build/block-library/blocks/comment-content/style-rtl.css 92 B
build/block-library/blocks/comment-content/style.css 92 B
build/block-library/blocks/comment-template/style-rtl.css 199 B
build/block-library/blocks/comment-template/style.css 198 B
build/block-library/blocks/comments-pagination-numbers/editor-rtl.css 123 B
build/block-library/blocks/comments-pagination-numbers/editor.css 121 B
build/block-library/blocks/comments-pagination/editor-rtl.css 222 B
build/block-library/blocks/comments-pagination/editor.css 209 B
build/block-library/blocks/comments-pagination/style-rtl.css 235 B
build/block-library/blocks/comments-pagination/style.css 231 B
build/block-library/blocks/comments-title/editor-rtl.css 75 B
build/block-library/blocks/comments-title/editor.css 75 B
build/block-library/blocks/comments/editor-rtl.css 840 B
build/block-library/blocks/comments/editor.css 839 B
build/block-library/blocks/comments/style-rtl.css 637 B
build/block-library/blocks/comments/style.css 636 B
build/block-library/blocks/cover/editor-rtl.css 647 B
build/block-library/blocks/cover/editor.css 650 B
build/block-library/blocks/cover/style-rtl.css 1.61 kB
build/block-library/blocks/cover/style.css 1.6 kB
build/block-library/blocks/details/editor-rtl.css 65 B
build/block-library/blocks/details/editor.css 65 B
build/block-library/blocks/embed/editor-rtl.css 293 B
build/block-library/blocks/embed/editor.css 293 B
build/block-library/blocks/embed/style-rtl.css 410 B
build/block-library/blocks/embed/style.css 410 B
build/block-library/blocks/embed/theme-rtl.css 126 B
build/block-library/blocks/embed/theme.css 126 B
build/block-library/blocks/file/editor-rtl.css 316 B
build/block-library/blocks/file/editor.css 316 B
build/block-library/blocks/file/style-rtl.css 269 B
build/block-library/blocks/file/style.css 270 B
build/block-library/blocks/file/view.min.js 312 B
build/block-library/blocks/footnotes/style-rtl.css 200 B
build/block-library/blocks/footnotes/style.css 199 B
build/block-library/blocks/freeform/editor-rtl.css 2.58 kB
build/block-library/blocks/freeform/editor.css 2.58 kB
build/block-library/blocks/gallery/editor-rtl.css 947 B
build/block-library/blocks/gallery/editor.css 952 B
build/block-library/blocks/gallery/style-rtl.css 1.53 kB
build/block-library/blocks/gallery/style.css 1.53 kB
build/block-library/blocks/gallery/theme-rtl.css 108 B
build/block-library/blocks/gallery/theme.css 108 B
build/block-library/blocks/group/editor-rtl.css 654 B
build/block-library/blocks/group/editor.css 654 B
build/block-library/blocks/group/style-rtl.css 57 B
build/block-library/blocks/group/style.css 57 B
build/block-library/blocks/group/theme-rtl.css 78 B
build/block-library/blocks/group/theme.css 78 B
build/block-library/blocks/heading/style-rtl.css 76 B
build/block-library/blocks/heading/style.css 76 B
build/block-library/blocks/html/editor-rtl.css 336 B
build/block-library/blocks/html/editor.css 337 B
build/block-library/blocks/image/editor-rtl.css 834 B
build/block-library/blocks/image/editor.css 833 B
build/block-library/blocks/image/style-rtl.css 1.42 kB
build/block-library/blocks/image/style.css 1.42 kB
build/block-library/blocks/image/theme-rtl.css 126 B
build/block-library/blocks/image/theme.css 126 B
build/block-library/blocks/image/view-interactivity.min.js 1.46 kB
build/block-library/blocks/latest-comments/style-rtl.css 357 B
build/block-library/blocks/latest-comments/style.css 357 B
build/block-library/blocks/latest-posts/editor-rtl.css 213 B
build/block-library/blocks/latest-posts/editor.css 212 B
build/block-library/blocks/latest-posts/style-rtl.css 478 B
build/block-library/blocks/latest-posts/style.css 478 B
build/block-library/blocks/list/style-rtl.css 88 B
build/block-library/blocks/list/style.css 88 B
build/block-library/blocks/media-text/editor-rtl.css 266 B
build/block-library/blocks/media-text/editor.css 263 B
build/block-library/blocks/media-text/style-rtl.css 507 B
build/block-library/blocks/media-text/style.css 505 B
build/block-library/blocks/more/editor-rtl.css 431 B
build/block-library/blocks/more/editor.css 431 B
build/block-library/blocks/navigation-link/editor-rtl.css 712 B
build/block-library/blocks/navigation-link/editor.css 711 B
build/block-library/blocks/navigation-link/style-rtl.css 115 B
build/block-library/blocks/navigation-link/style.css 115 B
build/block-library/blocks/navigation-submenu/editor-rtl.css 296 B
build/block-library/blocks/navigation-submenu/editor.css 295 B
build/block-library/blocks/navigation/editor-rtl.css 2.26 kB
build/block-library/blocks/navigation/editor.css 2.26 kB
build/block-library/blocks/navigation/style-rtl.css 2.21 kB
build/block-library/blocks/navigation/style.css 2.2 kB
build/block-library/blocks/navigation/view.min.js 984 B
build/block-library/blocks/nextpage/editor-rtl.css 395 B
build/block-library/blocks/nextpage/editor.css 395 B
build/block-library/blocks/page-list/editor-rtl.css 401 B
build/block-library/blocks/page-list/editor.css 401 B
build/block-library/blocks/page-list/style-rtl.css 175 B
build/block-library/blocks/page-list/style.css 175 B
build/block-library/blocks/paragraph/editor-rtl.css 174 B
build/block-library/blocks/paragraph/editor.css 174 B
build/block-library/blocks/paragraph/style-rtl.css 279 B
build/block-library/blocks/paragraph/style.css 281 B
build/block-library/blocks/post-author/style-rtl.css 175 B
build/block-library/blocks/post-author/style.css 176 B
build/block-library/blocks/post-comments-form/editor-rtl.css 96 B
build/block-library/blocks/post-comments-form/editor.css 96 B
build/block-library/blocks/post-comments-form/style-rtl.css 508 B
build/block-library/blocks/post-comments-form/style.css 508 B
build/block-library/blocks/post-date/style-rtl.css 61 B
build/block-library/blocks/post-date/style.css 61 B
build/block-library/blocks/post-excerpt/editor-rtl.css 71 B
build/block-library/blocks/post-excerpt/editor.css 71 B
build/block-library/blocks/post-excerpt/style-rtl.css 141 B
build/block-library/blocks/post-excerpt/style.css 141 B
build/block-library/blocks/post-featured-image/editor-rtl.css 588 B
build/block-library/blocks/post-featured-image/editor.css 586 B
build/block-library/blocks/post-featured-image/style-rtl.css 319 B
build/block-library/blocks/post-featured-image/style.css 319 B
build/block-library/blocks/post-navigation-link/style-rtl.css 153 B
build/block-library/blocks/post-navigation-link/style.css 153 B
build/block-library/blocks/post-template/editor-rtl.css 99 B
build/block-library/blocks/post-template/editor.css 98 B
build/block-library/blocks/post-template/style-rtl.css 314 B
build/block-library/blocks/post-template/style.css 314 B
build/block-library/blocks/post-terms/style-rtl.css 96 B
build/block-library/blocks/post-terms/style.css 96 B
build/block-library/blocks/post-time-to-read/style-rtl.css 69 B
build/block-library/blocks/post-time-to-read/style.css 69 B
build/block-library/blocks/post-title/style-rtl.css 100 B
build/block-library/blocks/post-title/style.css 100 B
build/block-library/blocks/preformatted/style-rtl.css 103 B
build/block-library/blocks/preformatted/style.css 103 B
build/block-library/blocks/pullquote/editor-rtl.css 135 B
build/block-library/blocks/pullquote/editor.css 135 B
build/block-library/blocks/pullquote/style-rtl.css 335 B
build/block-library/blocks/pullquote/style.css 335 B
build/block-library/blocks/pullquote/theme-rtl.css 167 B
build/block-library/blocks/pullquote/theme.css 167 B
build/block-library/blocks/query-pagination-numbers/editor-rtl.css 122 B
build/block-library/blocks/query-pagination-numbers/editor.css 121 B
build/block-library/blocks/query-pagination/editor-rtl.css 221 B
build/block-library/blocks/query-pagination/editor.css 211 B
build/block-library/blocks/query-title/style-rtl.css 63 B
build/block-library/blocks/query-title/style.css 63 B
build/block-library/blocks/query/editor-rtl.css 450 B
build/block-library/blocks/query/editor.css 449 B
build/block-library/blocks/quote/style-rtl.css 222 B
build/block-library/blocks/quote/style.css 222 B
build/block-library/blocks/quote/theme-rtl.css 223 B
build/block-library/blocks/quote/theme.css 226 B
build/block-library/blocks/read-more/style-rtl.css 132 B
build/block-library/blocks/read-more/style.css 132 B
build/block-library/blocks/rss/editor-rtl.css 149 B
build/block-library/blocks/rss/editor.css 149 B
build/block-library/blocks/rss/style-rtl.css 289 B
build/block-library/blocks/rss/style.css 288 B
build/block-library/blocks/search/editor-rtl.css 178 B
build/block-library/blocks/search/editor.css 178 B
build/block-library/blocks/search/style-rtl.css 587 B
build/block-library/blocks/search/style.css 584 B
build/block-library/blocks/search/theme-rtl.css 114 B
build/block-library/blocks/search/theme.css 114 B
build/block-library/blocks/search/view.min.js 531 B
build/block-library/blocks/separator/editor-rtl.css 146 B
build/block-library/blocks/separator/editor.css 146 B
build/block-library/blocks/separator/style-rtl.css 234 B
build/block-library/blocks/separator/style.css 234 B
build/block-library/blocks/separator/theme-rtl.css 194 B
build/block-library/blocks/separator/theme.css 194 B
build/block-library/blocks/shortcode/editor-rtl.css 323 B
build/block-library/blocks/shortcode/editor.css 323 B
build/block-library/blocks/site-logo/editor-rtl.css 754 B
build/block-library/blocks/site-logo/editor.css 754 B
build/block-library/blocks/site-logo/style-rtl.css 203 B
build/block-library/blocks/site-logo/style.css 203 B
build/block-library/blocks/site-tagline/editor-rtl.css 86 B
build/block-library/blocks/site-tagline/editor.css 86 B
build/block-library/blocks/site-title/editor-rtl.css 116 B
build/block-library/blocks/site-title/editor.css 116 B
build/block-library/blocks/site-title/style-rtl.css 57 B
build/block-library/blocks/site-title/style.css 57 B
build/block-library/blocks/social-link/editor-rtl.css 184 B
build/block-library/blocks/social-link/editor.css 184 B
build/block-library/blocks/social-links/editor-rtl.css 674 B
build/block-library/blocks/social-links/editor.css 673 B
build/block-library/blocks/social-links/style-rtl.css 1.43 kB
build/block-library/blocks/social-links/style.css 1.42 kB
build/block-library/blocks/spacer/editor-rtl.css 348 B
build/block-library/blocks/spacer/editor.css 348 B
build/block-library/blocks/spacer/style-rtl.css 48 B
build/block-library/blocks/spacer/style.css 48 B
build/block-library/blocks/table/editor-rtl.css 433 B
build/block-library/blocks/table/editor.css 433 B
build/block-library/blocks/table/style-rtl.css 645 B
build/block-library/blocks/table/style.css 644 B
build/block-library/blocks/table/theme-rtl.css 146 B
build/block-library/blocks/table/theme.css 146 B
build/block-library/blocks/tag-cloud/style-rtl.css 251 B
build/block-library/blocks/tag-cloud/style.css 253 B
build/block-library/blocks/template-part/editor-rtl.css 403 B
build/block-library/blocks/template-part/editor.css 403 B
build/block-library/blocks/template-part/theme-rtl.css 101 B
build/block-library/blocks/template-part/theme.css 101 B
build/block-library/blocks/term-description/style-rtl.css 111 B
build/block-library/blocks/term-description/style.css 111 B
build/block-library/blocks/text-columns/editor-rtl.css 95 B
build/block-library/blocks/text-columns/editor.css 95 B
build/block-library/blocks/text-columns/style-rtl.css 166 B
build/block-library/blocks/text-columns/style.css 166 B
build/block-library/blocks/verse/style-rtl.css 99 B
build/block-library/blocks/verse/style.css 99 B
build/block-library/blocks/video/editor-rtl.css 552 B
build/block-library/blocks/video/editor.css 555 B
build/block-library/blocks/video/style-rtl.css 174 B
build/block-library/blocks/video/style.css 174 B
build/block-library/blocks/video/theme-rtl.css 126 B
build/block-library/blocks/video/theme.css 126 B
build/block-library/classic-rtl.css 179 B
build/block-library/classic.css 179 B
build/block-library/common-rtl.css 1.1 kB
build/block-library/common.css 1.1 kB
build/block-library/editor-elements-rtl.css 75 B
build/block-library/editor-elements.css 75 B
build/block-library/editor-rtl.css 12.1 kB
build/block-library/editor.css 12.1 kB
build/block-library/elements-rtl.css 54 B
build/block-library/elements.css 54 B
build/block-library/reset-rtl.css 478 B
build/block-library/reset.css 478 B
build/block-library/theme-rtl.css 686 B
build/block-library/theme.css 691 B
build/block-serialization-default-parser/index.min.js 1.12 kB
build/block-serialization-spec-parser/index.min.js 2.87 kB
build/blocks/index.min.js 51 kB
build/commands/index.min.js 14.9 kB
build/commands/style-rtl.css 827 B
build/commands/style.css 827 B
build/components/index.min.js 240 kB
build/components/style-rtl.css 11.7 kB
build/components/style.css 11.7 kB
build/compose/index.min.js 12 kB
build/core-commands/index.min.js 2.26 kB
build/customize-widgets/index.min.js 12 kB
build/customize-widgets/style-rtl.css 1.46 kB
build/customize-widgets/style.css 1.45 kB
build/data-controls/index.min.js 640 B
build/data/index.min.js 8.28 kB
build/date/index.min.js 17.8 kB
build/deprecated/index.min.js 451 B
build/dom-ready/index.min.js 324 B
build/dom/index.min.js 4.63 kB
build/edit-post/classic-rtl.css 544 B
build/edit-post/classic.css 545 B
build/edit-post/index.min.js 35.5 kB
build/edit-post/style-rtl.css 7.58 kB
build/edit-post/style.css 7.57 kB
build/edit-widgets/index.min.js 16.9 kB
build/edit-widgets/style-rtl.css 4.52 kB
build/edit-widgets/style.css 4.52 kB
build/editor/index.min.js 45.4 kB
build/editor/style-rtl.css 3.58 kB
build/editor/style.css 3.58 kB
build/element/index.min.js 4.8 kB
build/escape-html/index.min.js 537 B
build/format-library/index.min.js 7.62 kB
build/format-library/style-rtl.css 554 B
build/format-library/style.css 553 B
build/hooks/index.min.js 1.55 kB
build/html-entities/index.min.js 448 B
build/i18n/index.min.js 3.58 kB
build/interactivity/index.min.js 10.2 kB
build/is-shallow-equal/index.min.js 527 B
build/keyboard-shortcuts/index.min.js 1.64 kB
build/keycodes/index.min.js 1.84 kB
build/list-reusable-blocks/index.min.js 2.18 kB
build/list-reusable-blocks/style-rtl.css 836 B
build/list-reusable-blocks/style.css 836 B
build/media-utils/index.min.js 2.9 kB
build/notices/index.min.js 948 B
build/plugins/index.min.js 1.77 kB
build/preferences-persistence/index.min.js 1.84 kB
build/preferences/index.min.js 1.24 kB
build/primitives/index.min.js 943 B
build/priority-queue/index.min.js 1.52 kB
build/private-apis/index.min.js 943 B
build/react-i18n/index.min.js 615 B
build/react-refresh-runtime/index.min.js 7.31 kB
build/redux-routine/index.min.js 2.7 kB
build/reusable-blocks/index.min.js 2.54 kB
build/reusable-blocks/style-rtl.css 243 B
build/reusable-blocks/style.css 243 B
build/rich-text/index.min.js 10.9 kB
build/router/index.min.js 1.77 kB
build/server-side-render/index.min.js 1.94 kB
build/shortcode/index.min.js 1.39 kB
build/style-engine/index.min.js 1.83 kB
build/token-list/index.min.js 582 B
build/url/index.min.js 3.57 kB
build/vendors/inert-polyfill.min.js 2.48 kB
build/vendors/react-dom.min.js 41.8 kB
build/vendors/react.min.js 4.02 kB
build/viewport/index.min.js 958 B
build/warning/index.min.js 268 B
build/widgets/index.min.js 7.16 kB
build/widgets/style-rtl.css 1.15 kB
build/widgets/style.css 1.16 kB
build/wordcount/index.min.js 1.02 kB

compressed-size-action

@github-actions
Copy link

github-actions bot commented Jul 3, 2023

Flaky tests detected in 8f9041a.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5506558379
📝 Reported issues:

@ellatrix ellatrix force-pushed the fix/get-rich-text-values branch 2 times, most recently from ab7ac1b to 630a191 Compare July 4, 2023 06:47
@ellatrix ellatrix added the [Release] Do Not Punt Used to indicate the issue or pull request must not be moved from the assigned milestone label Jul 5, 2023
@ellatrix ellatrix force-pushed the fix/get-rich-text-values branch 2 times, most recently from ab2c3a3 to 28551f6 Compare July 7, 2023 18:57
@bph bph added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Jul 9, 2023
Copy link
Contributor

@andrewserong andrewserong left a comment

Choose a reason for hiding this comment

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

This is testing nicely for me, thanks for the explanation as to the similarity with renderToString. I'm not too familiar with this part of how rich text works, but the reasoning all sounds good to me, and this fixes up the issue nicely!

Before: adding the footnote in the List block does not update the Footnotes block to correctly reflect the newly added footnote. After: the Footnotes block is updated to reflect the newly added footnote:

Before After
2023-07-10 12 34 08 2023-07-10 12 34 52

LGTM! ✨

Copy link
Contributor

@mcsf mcsf left a comment

Choose a reason for hiding this comment

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

This is looking pretty good, thanks for the quick turnaround!

I do worry about the performance implications of running getRichTextValues on all blocks on every keystroke when all we need is to detect a change in the order of the footnotes. It seems to me we can easily improve things by breaking down the problem as follows:

BEFORE:

- On any change to blocks, run:
    content := getRichTextValues( blocks )
- Search content for footnote anchors
    newOrder := anchors.map( ( { id } ) => id )
- Compare new order with old
AFTER:

- Add cache to getRichTextValues, perhaps with a WeakMap
- On any change to blocks, run:
    contentPieces = blocks.map( getRichTextValues )
    (i.e. split the work and leverage the cache)
- Potentially add caching to the next stage (findAnchors) too:
    newOrder = contentPieces.flatMap( findAnchors )
- Compare new order with old

We can investigate this in a follow-up PR, though.

test/e2e/specs/editor/various/footnotes.spec.js Outdated Show resolved Hide resolved
const values = [];
addValuesForBlocks( values, blocks );
getBlockProps.skipFilters = false;
return values;
Copy link
Contributor

@draganescu draganescu Jul 10, 2023

Choose a reason for hiding this comment

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

Since values is a const [] this code reads as if we always return []?

Copy link
Contributor

Choose a reason for hiding this comment

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

addValuesForBlocks and subsequent calls all mutate that single array. Personally, I find it clear enough in the context of those four lines (otherwise values would be totally inert), and it's consistent with other performance-minded functions in rich text.

@mcsf mcsf merged commit 7021558 into trunk Jul 10, 2023
@mcsf mcsf deleted the fix/get-rich-text-values branch July 10, 2023 11:32
ockham pushed a commit that referenced this pull request Jul 10, 2023
…ent (#52241)

* RichText/Footnotes: make getRichTextValues work with InnerBlocks.Content

---------

Co-authored-by: Miguel Fonseca <150562+mcsf@users.noreply.github.com>
@ockham
Copy link
Contributor

ockham commented Jul 10, 2023

Cherry-picked into the release/16.2 branch in 0bde5ae.

tellthemachines pushed a commit that referenced this pull request Jul 11, 2023
…ent (#52241)

* RichText/Footnotes: make getRichTextValues work with InnerBlocks.Content

---------

Co-authored-by: Miguel Fonseca <150562+mcsf@users.noreply.github.com>
@tellthemachines tellthemachines removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Jul 11, 2023
tellthemachines added a commit that referenced this pull request Jul 11, 2023
* Post and Comment Template blocks: Change render_block_context priority to 1 (#52364)

* Footnotes: fix lingering format boundary attr (#52439)

* Footnotes: Fix incorrect anchor position in Firefox (#52425)

* Scope CSS rules for the wp admin reset to js support only. (#52376)

* Fix: Patterns & template parts: remove "apply globally" option from block settings (#52160)

* Advanced styles panel: add an early return

* Update index.js

* Minor styling changes

* Use array of features

---------

Co-authored-by: George Mamadashvili <georgemamadashvili@gmail.com>

* make the body of the editor minimmum viewport height so that smaller contents maintain clickability (#52406)

* Patterns: Add renaming, duplication, and deletion options (#52270)

* Patterns: Update manage pattern links to go to site editor if available (#52403)

* [Patterns] Separate sync status into a filter control (#52303)

Co-authored-by: Saxon Fletcher <saxonafletcher@gmail.com>
Co-authored-by: Glen Davies <glen.davies@automattic.com>

* Patterns: Add missing decoding entities processing in Patterns and Template/Parts pages (#52449)

* Fix document title icon appearance (#52424)

* Quote block: Add transform to paragraph (#51809)

* Add ungroup transform as transform to p

* Lint

* Update test and snapshot.

---------

Co-authored-by: Juan Aldasoro <juanfraa@gmail.com>

* remove sidebar group descriptions (#52453)

* Patterns:  alternative grid layout to improve keyboard accessibility (#52357)

* add sync tooltip (#52458)

* Patterns: Update section heading levels (#52273)

* Ensure that the unsaved title is not persisted when reopening the modal (#52473)

* Iframe: avoid asset parsing & fix script localisation (#52405)

* Iframe: avoid asset parsing & fix script localisation

* Add e2e test for script localisation

* Update descriptions (#52468)

* Footnotes: show in inserter and placehold (#52445)

* Footnotes: show in inserter and placehold

* Fix placeholder block membrane; fix copy; add icon, label

---------

Co-authored-by: Miguel Fonseca <150562+mcsf@users.noreply.github.com>

* Fix: Focus loss on navigation link label editing on Firefox. (#52428)

* Update tooltip (#52465)

* Refactor, document, and fix image block deprecations (#52081)

* Refactor, document, and fix image block deprecations

* Fix v5 attributes & supports

* Fix v1 & v2 deprecation tests

* Rename deprecated test descriptions

* Respect custom aspect ratio (#52286)

* add image width and height via css inline style, update width and height attrs to be string

* keep width and height as attributes too, keep the attributes as numbers

* updates image fixtures

* RichText/Footnotes: make getRichTextValues work with InnerBlocks.Content (#52241)

* RichText/Footnotes: make getRichTextValues work with InnerBlocks.Content

---------

Co-authored-by: Miguel Fonseca <150562+mcsf@users.noreply.github.com>

* Footnotes: save numbering through the entity provider (#52423)

* Footnotes: save numbering through the entity provider

* Add sup so no styling is needed at all

* Migrate old format

* Restore old styles, fix nested attribute queries

* Fix anchor selection

* Migrate markup in entity provider instead

* Fix tests

* Fix typo

* Fix comment

---------

Co-authored-by: Miguel Fonseca <150562+mcsf@users.noreply.github.com>

* Revert "Post editor: Require confirmation before removing Footnotes (#52277)" (#52486)

This reverts commit e6426ea.

* List block: Fix selected type option (#52472)

* Library - make pattern title clickable (#51898)

* Use button inside title

* Remove href

* Preserve roving tab index

* Fix link colors to match trunk $gray-600

* Remove redundant var

* Amend colors as per review

* remove old files again

---------

Co-authored-by: scruffian <ben@scruffian.com>

* remove status icon (#52457)

* Rename block theme activation nonce variable. (#52398)

---------

Co-authored-by: Bernie Reiter <96308+ockham@users.noreply.github.com>
Co-authored-by: Ella <4710635+ellatrix@users.noreply.github.com>
Co-authored-by: Aki Hamano <54422211+t-hamano@users.noreply.github.com>
Co-authored-by: Andrea Fercia <a.fercia@gmail.com>
Co-authored-by: Carolina Nymark <myazalea@hotmail.com>
Co-authored-by: George Mamadashvili <georgemamadashvili@gmail.com>
Co-authored-by: Andrei Draganescu <me@andreidraganescu.info>
Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com>
Co-authored-by: Kai Hao <kevin830726@gmail.com>
Co-authored-by: Saxon Fletcher <saxonafletcher@gmail.com>
Co-authored-by: Glen Davies <glen.davies@automattic.com>
Co-authored-by: James Koster <james@jameskoster.co.uk>
Co-authored-by: Rich Tabor <hi@richtabor.com>
Co-authored-by: Juan Aldasoro <juanfraa@gmail.com>
Co-authored-by: Miguel Fonseca <150562+mcsf@users.noreply.github.com>
Co-authored-by: Jorge Costa <jorge.costa@developer.pt>
Co-authored-by: Alex Lende <alex+github.com@lende.xyz>
Co-authored-by: Héctor <27339341+priethor@users.noreply.github.com>
Co-authored-by: Petter Walbø Johnsgård <petter@dekode.no>
Co-authored-by: Dave Smith <getdavemail@gmail.com>
Co-authored-by: scruffian <ben@scruffian.com>
Co-authored-by: Peter Wilson <519727+peterwilsoncc@users.noreply.github.com>
@mcsf
Copy link
Contributor

mcsf commented Jul 12, 2023

I do worry about the performance implications of running getRichTextValues on all blocks on every keystroke when all we need is to detect a change in the order of the footnotes. It seems to me we can easily improve things by breaking down the problem as follows […]
We can investigate this in a follow-up PR, though.

Opened #52577

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Footnotes Affects the Footnotes Block [Release] Do Not Punt Used to indicate the issue or pull request must not be moved from the assigned milestone [Type] Bug An existing feature does not function as intended
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Footnotes Block: unable to insert footnote on Paragraph inside Quote Block
7 participants