1175 fix spark notification position#23198
1175 fix spark notification position#23198FAMarfuaty merged 6 commits intofeature/content-plannerfrom
Conversation
After positioning the spark notification outside the panel.
Coverage Report for CI Build 5Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Coverage increased (+0.001%) to 53.476%Details
Uncovered Changes
Coverage Regressions40 previously-covered lines in 5 files lost coverage.
Coverage Stats💛 - Coveralls |
There was a problem hiding this comment.
Pull request overview
Adjusts the “spark” (SparksLimitNotification) notification positioning so it consistently appears at the bottom-left of the viewport when using the AI Generator and the Content Planner modal flows.
Changes:
- Add dynamic
bottompositioning forNotificationsbased on measured modal/panel height. - Introduce measuring of the Content Planner modal panel via
useMeasuredRefto compute notification offset. - Update Content Planner FeatureModal tests to mock
useMeasuredRef.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/js/tests/ai-content-planner/components/feature-modal.test.js | Mocks useMeasuredRef to keep tests stable after introducing resize-measure behavior. |
| packages/js/src/ai-generator/components/modal-content.js | Adds bottom offset computation and updates Notifications inline styles for viewport positioning. |
| packages/js/src/ai-content-planner/components/feature-modal.js | Measures modal panel height and applies computed bottom offset to Notifications in the Content Planner modal. |
| const margin = `calc(${ currentHeight === 0 ? "50%" : currentHeight / 2 + "px" } - 40vh)`; | ||
| const bottom = `calc( (${ height + 55 + "px" } - 100vh) / 2 )`; |
There was a problem hiding this comment.
Is this intentional that bottom uses non-dynamic height? If yes, I think it's useful to leave a code comment to document the decision.
Additionally, 55 seems like a magic number which is unexplained and undocumented. Consider creating a constant for this variable as it's also used inside feature-modal.js
There was a problem hiding this comment.
I created a helper function for that, added comments and removed the marginTop which was not needed anymore.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Context
Summary
This PR can be summarized in the following changelog entry:
Relevant technical choices:
Test instructions
Test instructions for the acceptance test before the PR gets merged
This PR can be acceptance tested by following these steps:
Relevant test scenarios
Test instructions for QA when the code is in the RC
QA can test this PR by following these steps:
Impact check
This PR affects the following parts of the plugin, which may require extra testing:
Other environments
[shopify-seo], added test instructions for Shopify and attached theShopifylabel to this PR.[yoast-doc-extension], added test instructions for Yoast SEO for Google Docs and attached theGoogle Docs Add-onlabel to this PR.Documentation
Quality assurance
grunt build:imagesand commited the results, if my PR introduces new images or SVGs.Innovation
innovationlabel.Fixes Fix spark notification position