-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Footnotes: try with post meta #51201
Conversation
Size Change: +2.36 kB (0%) Total Size: 1.43 MB
ℹ️ View Unchanged
|
Flaky tests detected in eae27827154ae660ad6d8d9a349e4db7001e9840. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5331673184
|
f4c7a69
to
100c0f7
Compare
@ellatrix I will be running the 16.0 RC1 release process in a few hours. Are you still trying to get this into 16.0 or can we bump to 16.1? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking generally good to me. Curious what you think about my comments? I'm being a bit annoying with tests :P
); | ||
} | ||
|
||
export function getRichTextValues( blocks = [] ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this function do, can we add some unit test to it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added some e2e tests, I think that's more valuable
// into view when it receives focus. | ||
onFocus={ ( event ) => { | ||
if ( ! event.target.textContent.trim() ) { | ||
event.target.scrollIntoView(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this really needed? I thought the browser scrolls when you focus something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, see above comment, I don't know why this is happening yet
7bd2069
to
3b89e82
Compare
Thank you! |
* wip * wip * wip * higher order reducer * Make links interactive * min diff * Add style * wip * Lock apis * Fix failing tests * Add fn block fixture * Clean up hor * Fix unit test * Don't save ce=false * Fix selection issues * Remove editor style in bock json * Footnotes: try with post meta * Clean up * Add server site render callback * Add fn block if not present * Fix scroll into view * Fix tests * Fix PHP linting * Guard for no meta * clean up * Fix unit tests * Fix package lock * Temporarily use __unstable * Don't do anything if fn is not supported * Add tests and comments * Fix php lint * Fix removal and test * Writing flow: ignore ce false on mouse leave * Insert footnote after selection instead of replacing it * Somewhat fix copy/cut/paste * Keep old footnotes so they can be restored on paste * Address feedback: flatMap * Prefer BFS search over flattenBlocks This should make a difference in performance for large enough posts. * Share RichText.Content with native * Try private api * Try removing private api from native * Try unlocking just in time * Skip JSON.parse for empty string * Fix perf issue --------- Co-authored-by: Miguel Fonseca <miguelcsf@gmail.com>
}; | ||
|
||
// Would be good to remove the format and HoR if the block is unregistered. | ||
registerFormatType( formatName, format ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we move this into the init
function instead?
This is causing an API request in alternative block editors (see #53874 (comment) ) even if the block is not registered.
What?
Previously: #50124, #49797, #47682, #28261, #15267.
Similar to #50124, but storing it with post meta.
Many approaches have been explored, including storing footnotes inline and in the footnotes block, but eventually we decided to go for storing them in post meta to make it easier to render the footnotes outside of the post content in templates.
To do: fix batching (#51644)
Why?
How?
This PR implements footnotes by:
Testing Instructions
Create a post, add a paragraph and click the "more" section in the rich text section to insert a new footnote anchor. A footnotes block should appear at the bottom where you can add the note. Add a few in different places, rearrange them, and check that the order at the bottom remains correct. Also test copy pasting.
Testing Instructions for Keyboard
Screenshots or screencast