Polish editor PostFormat and PostFormatPanel components. #24079
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ZebulanStanphill
added
[Type] Code Quality
|
Size Change: -18 B (0%) Total Size: 1.05 MB
ℹ️ View Unchanged
|
ZebulanStanphill
force-pushed
the
update/polish-editor-post-format-component
branch
4 times, most recently
from
651a9ae to
fbc62c7
Compare
ZebulanStanphill
changed the title
ZebulanStanphill
force-pushed
the
update/polish-editor-post-format-component
branch
2 times, most recently
from
a536b68 to
63a2c07
Compare
ZebulanStanphill
force-pushed
the
update/polish-editor-post-format-component
branch
2 times, most recently
from
831ddf7 to
43fb27f
Compare
ZebulanStanphill
force-pushed
the
update/polish-editor-post-format-component
branch
from
43fb27f to
02a24e9
Compare
ZebulanStanphill
force-pushed
the
update/polish-editor-post-format-component
branch
from
02a24e9 to
7ca79f1
Compare
ZebulanStanphill
force-pushed
the
update/polish-editor-post-format-component
branch
2 times, most recently
from
33ef6a1 to
0845115
Compare
6 tasks
talldan
requested changes
Oct 16, 2020
There was a problem hiding this comment.
I've reviewed half of this, but as a reviewer I've found this refactor has too many logic changes and parts that won't work the same as before.
A good rule of thumb for a refactor is that it should unit test the same as it did previously (if it had tests).
How about an intermediate PR where we refactor the existing component to hooks without changing logic?
|
Thanks for doing that. Merged that one just now. What's the next step? |
ZebulanStanphill
force-pushed
the
update/polish-editor-post-format-component
branch
3 times, most recently
from
ab71abb to
0ce47ba
Compare
ZebulanStanphill
changed the title
ZebulanStanphill
force-pushed
the
update/polish-editor-post-format-component
branch
2 times, most recently
from
021ea6c to
2f4de98
Compare
ZebulanStanphill
force-pushed
the
update/polish-editor-post-format-component
branch
from
2f4de98 to
589ff78
Compare
ZebulanStanphill
force-pushed
the
update/polish-editor-post-format-component
branch
from
589ff78 to
7128baa
Compare
ZebulanStanphill
force-pushed
the
update/polish-editor-post-format-component
branch
from
7128baa to
cb172ac
Compare
ZebulanStanphill
force-pushed
the
update/polish-editor-post-format-component
branch
from
cb172ac to
1024849
Compare
ZebulanStanphill
force-pushed
the
update/polish-editor-post-format-component
branch
from
1024849 to
8aa02d3
Compare
|
Ultimately, this PR is basically just nitpicking, and most of the more useful changes have made it into |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
This PR refactors two components in the
editorpackage:PostFormatandPostFormatPanel. The changes made are designed to simplify the code and make it easier to implement changes I want to make in #24024.List of changes
undefined/nullhandling using nullish coalescing and native JS array methods.PostFormatPanel'suseSelectcallback to return only a single value.PostFormatPanel'sPostFormatSuggestionto take a single argument for the callback, rather than one for a callback and one for a value to pass to that callback.PostFormatPanel's containedPanelBodyto use aFragmentfor its title element, rather than an array of elements withkeyprops.PostFormatandPostFormatPanelto move a lot of the logic into theuseSelectcallbacks, to avoid unnecessarily running the same checks on every render.Checklist: