Fix summary toolbar positioning and selection on blur #38086
Conversation
|
Hi , @woocommerce/mothra Apart from reviewing the code changes, please make sure to review the testing instructions as well. You can follow this guide to find out what good testing instructions should look like: |
Test Results SummaryCommit SHA: 2dce869
To view the full API test report, click here. To view the full E2E test report, click here. To view all test reports, visit the WooCommerce Test Reports Dashboard. |
mdperez86
left a comment
mdperez86
left a comment
There was a problem hiding this comment.
After doing some local testing I found a rare behavior with the toolbar. Maybe that is not gonna happened to a merchant.
When testing with the DevTool opened and changing the focus between the Summary field and the DevTool window, for some reason the toolbar only shows one button item. But if we change the focus between element within the form the tool bar shows all the items in this case with a little delay between each item render.
Screen.Recording.2023-05-03.at.12.12.48.PM.mov
| style: { direction }, | ||
| } ); | ||
| const id = uniqueId(); | ||
| const contentId = uniqueId( |
There was a problem hiding this comment.
Since there are future plans to remove lodash from WP and given that there is a similar function then we should use import { useInstanceId } from '@wordpress/compose'; instead as the standard right!?
| dir={ direction } | ||
| allowedFormats={ allowedFormats } | ||
| /> | ||
| <div { ...blockProps }> |
There was a problem hiding this comment.
Do you think that moving this from the top most element is not gonna cause any rare behavior in the future?
I'm just curious about it.
There was a problem hiding this comment.
I was wondering the same. It might cause some issues in the future, but it's pretty easy to revert if that happens.
If that turns out to be the case, we may try creating a class in the editor root to determine current block selection and position the toolbar accordingly. This is not an ideal solution, but currently there is no way to pass any props to the toolbar popover.
I'm able to see this too and believe it's related to Gutenberg behavior around selecting the parent block. This is also visible on
Since this is pretty hard to reproduce on this branch without having dev tools open and is easily resolved by selecting text you want to modify, I'm okay to leave this as is unless you think it's a priority. |
Submission Review Guidelines:
Changes proposed in this Pull Request:
Closes #37956 .
How to test the changes in this Pull Request:
Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions: