Skip to content
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

[Experimental] [CYS - Full Composability]: Add tooltip when user clicks on pattern #47583

Merged

Conversation

gigitux
Copy link
Contributor

@gigitux gigitux commented May 17, 2024

Submission Review Guidelines:

Changes proposed in this Pull Request:

This PR adds a tooltip when the user clicks on a pattern.

Closes #47455 #47453.

This PR adds the tooltip when the user clicks on a pattern. This work required some refactoring, given that I had to optimize the functions that are called during each render to avoid a sluggish user experience.

usePopoverHandler

I added usePopoverHandle hook to handle the computation of the position of the popover based on the pointer

useAddAutoBlockPreviewEventListenersAndObservers

I added useAddAutoBlockPreviewEventListenersAndObservers to centralize all the hooks and mutationObservers in one place. Furthermore, I ensured that mutation observers are called only when necessary, given that they hurt performance (example: https://github.com/woocommerce/woocommerce/blob/693b153fc00ffbb9d9758f69c3e81c16c28c38a3/plugins/woocommerce-admin/client/customize-store/assembler-hub/hooks/auto-block-preview-event-listener.ts/#L258-L259).

Also, I ensured that we unsubscribe them when the component is unmounted. This is really important because the AutoHeightBlockPreview component is used for the main iframe and the small previews in the sidebar.

Futhermore, I created two new functions:

Removed not used code

Removed unused code related to the "other pages" feature, which wasn't shipped in the initial CYS version: https://github.com/woocommerce/woocommerce/pull/47583/files#diff-2320f0b44212379f7b24fd69a40a3b7ab6be829ea02c507d3c81de35c4948cc5L128-L165

How to test the changes in this Pull Request:

Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:

  1. Ensure that you have the last version of Gutenberg enabled
  2. Make sure you have the WooCommerce beta tester plugin installed
  3. Head over to Tools > WCA Test Helper > Features
  4. Make sure you see the pattern-toolkit-full-composability on the list.
  5. Enable it.
  6. Head over to WooCommerce > Home > Customize your store.
  7. On the main iframe, click on a block.
  8. Ensure that a tooltip appears.
  9. Move the mouse pointer and ensure that the tooltip follows the pointer.
  10. Hover another pattern.
  11. Ensure that the tooltip disappears.
  12. Click another pattern.
  13. Ensure that the tooltip position changes.
  14. Move the pointer on the left sidebar.
  15. Ensure that the tooltip disappears.
LofFz5.mp4

Changelog entry

  • Automatically create a changelog entry from the details below.

Significance

  • Patch
  • Minor
  • Major

Type

  • Fix - Fixes an existing bug
  • Add - Adds functionality
  • Update - Update existing functionality
  • Dev - Development related task
  • Tweak - A minor adjustment to the codebase
  • Performance - Address performance issues
  • Enhancement - Improvement to existing functionality

Message

CYS: Show popover when the user clicks on the pattern

Comment

@gigitux gigitux linked an issue May 17, 2024 that may be closed by this pull request
@gigitux gigitux changed the base branch from trunk to add/highlight-selected-block May 17, 2024 13:17
@github-actions github-actions bot added focus: monorepo infrastructure Issues and PRs related to monorepo tooling. plugin: woocommerce Issues related to the WooCommerce Core plugin. labels May 17, 2024
Copy link
Contributor

github-actions bot commented May 17, 2024

Test using WordPress Playground

The changes in this pull request can be previewed and tested using a WordPress Playground instance.
WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Test this pull request with WordPress Playground.

Note that this URL is valid for 30 days from when this comment was last updated. You can update it by closing/reopening the PR or pushing a new commit.

Base automatically changed from add/highlight-selected-block to trunk May 28, 2024 08:16
@github-actions github-actions bot removed focus: monorepo infrastructure Issues and PRs related to monorepo tooling. plugin: woocommerce Issues related to the WooCommerce Core plugin. labels May 30, 2024
Copy link
Contributor

github-actions bot commented May 31, 2024

Size Change: 0 B

Total Size: 2.47 MB

compressed-size-action

@gigitux gigitux force-pushed the 47455-cys-full-composability-add-tooltip-on-the-hovered-pattern branch from 02038a1 to 677cc56 Compare May 31, 2024 13:17
@gigitux gigitux closed this May 31, 2024
@gigitux gigitux reopened this May 31, 2024
@github-actions github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label May 31, 2024
…-pattern' of github.com:woocommerce/woocommerce into 47455-cys-full-composability-add-tooltip-on-the-hovered-pattern
@gigitux gigitux force-pushed the 47455-cys-full-composability-add-tooltip-on-the-hovered-pattern branch from 8e1ce74 to 2c5bc64 Compare May 31, 2024 13:52
@gigitux gigitux changed the title [CYS - Full Composability]: Add tooltip hovered pattern [Experimental] [CYS - Full Composability]: Add tooltip when user clicks on pattern Jun 3, 2024
@gigitux gigitux self-assigned this Jun 3, 2024
@gigitux gigitux added team: Kirigami & Origami focus: customize-your-store Issues related to the Customize Your Store onboarding flow. labels Jun 3, 2024
@gigitux gigitux marked this pull request as ready for review June 3, 2024 10:32
@gigitux gigitux requested review from albarin and nefeline June 3, 2024 10:32
@woocommercebot woocommercebot requested a review from a team June 3, 2024 10:32
Copy link
Contributor

github-actions bot commented Jun 3, 2024

Hi @albarin, @nefeline,

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:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

@gigitux
Copy link
Contributor Author

gigitux commented Jun 3, 2024

I noticed that enabling the mousemove event , the console throws multiple errors:

DCkwyz.mp4

This is caused by the useInBetweenInserter hook. This hook is necessary to show the inserter. In this case, we don't need the inserter feature, so it isn't a big deal. However, it could be noisy, so if you agree with me, I will open a dedicated issue when the PR is merged.

Also, I think that the quickest fix is to change this check to if ( openRef?.current ) {

Copy link
Member

@nefeline nefeline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this, @gigitux !

I've confirmed everything works as described:

Screen.Recording.2024-06-05.at.13.55.08.mov

Not a blocker, but a thought: whenever hovering over the sidebar and going back to the main frame, I see the tooltip is still displayed even without any new click on the pattern: should we change this to, if the pointer is moved away from the frame and then back, the tooltip is displayed again only on click?

@gigitux
Copy link
Contributor Author

gigitux commented Jun 5, 2024

Not a blocker, but a thought: whenever hovering over the sidebar and going back to the main frame, I see the tooltip is still displayed even without any new click on the pattern: should we change this to, if the pointer is moved away from the frame and then back, the tooltip is displayed again only on click?

Thanks for catching this, @nefeline!

I agree with you. Given that the PR is already very big, I'm going to create a dedicated issue to fix this!

@gigitux gigitux enabled auto-merge (squash) June 5, 2024 12:24
@gigitux gigitux merged commit 50b29d2 into trunk Jun 5, 2024
28 checks passed
@gigitux gigitux deleted the 47455-cys-full-composability-add-tooltip-on-the-hovered-pattern branch June 5, 2024 12:27
@github-actions github-actions bot added this to the 9.1.0 milestone Jun 5, 2024
@github-actions github-actions bot added the needs: analysis Indicates if the PR requires a PR testing scrub session. label Jun 5, 2024
@Stojdza Stojdza added needs: external testing Indicates if the PR requires further testing conducted by testers external to the development team. status: analysis complete Indicates if a PR has been analysed by Solaris and removed needs: analysis Indicates if the PR requires a PR testing scrub session. needs: external testing Indicates if the PR requires further testing conducted by testers external to the development team. labels Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
focus: customize-your-store Issues related to the Customize Your Store onboarding flow. plugin: woocommerce Issues related to the WooCommerce Core plugin. status: analysis complete Indicates if a PR has been analysed by Solaris team: Kirigami & Origami
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CYS - Full Composability]: Add tooltip on the hovered pattern
3 participants