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

Removing redundant custom transient notices. #872

Merged
merged 3 commits into from
Dec 27, 2023

Conversation

message-dimke
Copy link
Contributor

@message-dimke message-dimke commented Dec 26, 2023

Changes proposed in this Pull Request:

Closes #867 .

Removing TransientNotice component. Having it causes transient notice appear twice and Pinterest extension styles for the notice override WooCommerce global styles for other pages making notices shift left.

The reason for that is TransientNotice component and createNotice() JS API method both dispatch the same action, my personal belief is that TransientNotice was added because of a lack of knowledge or before createNotice() became available.

Removing the component shots two issues at once: double notice bug ( when at Pinterest extension pages ) and new experimental Product Editor notices shifted to the left.

Detailed test instructions:

Double notice issue.

Settings_‹_Pinterest_‹_Marketing_‹_WooCommerce_‹_WordPress_Pinterest_—_WooCommerce
  1. Install recent Pinterest plugin version and connect.
  2. Being at Settings tab click 'Sync' ( top right ).
  3. Observe the notice about sync status appears bottom left.
  4. Check with Web Inspector there are two notices on a page ( even visually it is darker than the one inside Product Editor, because there are actually two the same notices one above the other ). Also those notices aren't next to each other in the source code, so finding both occurrences requires some search while the notices are still visible.
  5. Checkout fix/remove-custom-transient-notices branch
  6. run nvm use && npm run build to have new sources built.
  7. Being at Settings tab click 'Sync' ( top right ).
  8. Check there is only a single notice copy on the page.

Experimental Product Editor transient notice shift.

To reproduce this one and test the fix you will need a wpcomstaging.com website.

  1. Setup the website.
  2. Enable experimental Product Editor.
WooCommerce_settings_‹_TheWayWeBook_—_WordPress
  1. Add new or update an existing product.

  2. Observe the transient notice bottom left. It must not overlap with the sidebar.

  3. Install Pinterest extension ( no need to connect to Pinterest, we are testing CSS only )

  4. Add new or update an existing product.

  5. Observe the transient notice bottom left. It overlaps with the sidebar.

‹_TheWayWeBook-2
  1. Checkout fix/remove-custom-transient-notices branch.
  2. Run nvm use && npm run build:zip to produce a ZIP file.
  3. Uninstall the existing Pinterest plugin and install the one out the ZIP file.
  4. Add or update a product using new experimental Product Editor.
  5. Observe the transient notice bottom left. It must not overlap with the sidebar.
‹_TheWayWeBook-3

Changelog entry

Fix - Product Editor transient notice shift.
Fix - Pinterest duplicated notices.

@message-dimke message-dimke added the type: bug The issue is a confirmed bug. label Dec 26, 2023
@message-dimke message-dimke self-assigned this Dec 26, 2023
@github-actions github-actions bot added the changelog: fix Took care of something that wasn't working. label Dec 26, 2023
@message-dimke message-dimke requested a review from a team December 26, 2023 14:41
@message-dimke message-dimke marked this pull request as ready for review December 26, 2023 20:35
Copy link
Collaborator

@budzanowski budzanowski left a comment

Choose a reason for hiding this comment

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

This is a great change :)

Just remove the .pot changes and this is good to go.

@message-dimke message-dimke merged commit 7702507 into develop Dec 27, 2023
3 checks passed
@message-dimke message-dimke deleted the fix/remove-custom-transient-notices branch December 27, 2023 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: fix Took care of something that wasn't working. type: bug The issue is a confirmed bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Notices styling is too generic - Affects all WooCommerce admin notices
2 participants