Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Adds Block Appender as placeholder to empty InnerBlocks #14241
Adds Block Appender as placeholder to empty InnerBlocks #14241
Changes from all commits
842e742
12e7f5c
4cf6051
ea1ea40
07557d1
5e648ec
6470b37
1d438c2
0a66f77
57a640e
88606c2
6f1baea
08dc914
f736575
30f0922
606896d
05a1dfc
f698f73
ee2b182
0b53217
536f425
4abf1f3
558771b
b1af612
56b87ef
1dc1b8b
a7f0eaa
4fc3918
78f64fc
10979e1
bd4e2ae
1df2ca0
f390c55
58cd20a
6a13d86
b26a5a6
d886e56
39ce905
0aaae76
faf147f
4c0ad0c
ff90334
d1efe5c
741ccd2
add695c
676bd1b
6d19945
b36e3a3
bb43af3
1dc1ec3
267626c
2a5e98b
e2f745a
6f4e295
13d243a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
On my prior point about being able to use the component as:
...it depends on treating the render prop as a component definition. It probably works for plain function components, but also might be more correctly expressed as:
createElement( renderAppender )
.I see most other resources which talk about render props ([1] [2]) don't really mention this, and in most cases it doesn't make a difference, except if you'd want to reimplement
ButtonBlockAppender
as a class component.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.
Interesting.
createElement( renderAppender )
would prevent any arguments being passed in the way render props typically do, though I suppose props could still be used.I've left it as it is for now for consistency with other render props in the codebase, but could be something to explore in the future.
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 the way I tend to see render props used anyways (passed a props object, if anything), so it confused me that there's a tendency to treat them as components by convention, but not through actual use of
createElement
.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.
Forgive me if I missed something about this, but why not an
IconButton
with alabel
prop instead of a Button with an icon and a hidden aria text.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.
Not sure, I wonder if it should also use an
aria-label
instead ofscreen-reader-text
.I had an attempt at replacing it with an IconButton, but that component has some high specificity on its hover styles that are hard to override.