Remove extraneous element from extensions HoC #2334
Conversation
jrtashjian
requested review from
EvanHerman,
AnthonyLedesma,
jonathanbardo and
olafleur-godaddy
as code owners
|
Test summary
Run details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
|
There is a single And here is the toolbar behavior in this PR. |
|
Ahh, that's right! Thanks for pointing this out. I'll see what I can do to restore the functionality while avoiding the extra element. |
olafleur-godaddy
removed request for
jonathanbardo,
EvanHerman and
olafleur-godaddy
|
@jrtashjian I just found a bug with the Normal Bug if you rush - hides the toolbar forever |
Performance Test Results:
|
|
Tested this in Go, TwentyTwenty, TwentyTwentyOne, and TwentyTwentyTwo. |
|
@AnthonyLedesma it looks like the performance tests indicate a decent degradation, should we be concerned about this? |
|
I guess I can't leave a review if I'm the original PR author. The issue still seems to be apparent. If you apply the changes you've made to the branch I wouldn't expect the style tag to cause a reflow but it seems that flex-box is considering it an element in-flow. I'm wondering if hiding the toolbar is really a benefit. It's obviously built to float around as necessary even if the anchor location changes. In my mind that's intended functionality. I'd prefer we just remove this style override so we have a stable content layout. Kapture.2022-05-17.at.10.58.53.mp4 |
At this point, I agree. It would be nice but the truth is that we accomplish it now in a very hacky way. I'll push up the changes that remove this extra feature and its implementation. |
|
✅ |
Remove extraFeature from animation as well as its implementation. This has been deemed unnecessary as the toolbar might be expected to shift as the block does.