-
Notifications
You must be signed in to change notification settings - Fork 904
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
FeatureUpsell story: Allow toggling upsell state within the story #20263
base: trunk
Are you sure you want to change the base?
Conversation
I think adding a story that is the same as the factory component is maybe redundant? |
A merge conflict has been detected for the proposed code changes in this PR. Please resolve the conflict by either rebasing the PR or merging in changes from the base branch. |
The FeatureUpsell component supports 2 variants in which it can render. It has a "default" variant and a "card" variant. When the variant is not specified, the FeatureUpsell component shows as the "card" variant. While that behaviour remains unchanged for the component (and is documented in the Storybook), the default Factory story does show the FeatureUpsell in its "default" variant style. A dedicated story to show the "card" variant style already exists.
8ff31cd
to
89aeb92
Compare
Pull Request Test Coverage Report for Build c7a0c4f5a3ac52d4cdda946d67ad2b9faa0b97baDetails
💛 - Coveralls |
Hey @vraja-pro, I found this old PR of mine again and decided to update it.
I agree. The component has a bit of a silly default. I've enforced the default variant in the factory story now in
I personally prefer the button to give that interactive feeling. We have similar buttons to toggle toasts and modals.
You're right. I've added additional explanatory copy behind the upsell to better tell (and show) what's happening in Clarify the behaviour op the locked content in a FeatureUpsell story. And I've removed the strange blue background color in preference of the primary button, which demonstrates the same effect in Realistic grayscale example. |
Context
FeatureUpsell
to switch between the upsell and non-upsell state.Summary
This PR can be summarized in the following changelog entry:
FeatureUpsell
to switch between the upsell and non-upsell state.Relevant technical choices:
args.shouldUpsell
to allow switching from the Storybook controls.onClick
to an<a>
tag, so that callback needsevent.preventDefault
etc.Test instructions
Test instructions for the acceptance test before the PR gets merged
This PR can be acceptance tested by following these steps:
$ yarn workspace @yoast/ui-library storybook
FeatureUpsell
component as expected.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:
UI changes
Other environments
[shopify-seo]
, added test instructions for Shopify and attached theShopify
label to this PR.Documentation
Quality assurance
Innovation
innovation
label and noted the work hours.Fixes Yoast/ui-library-storybook#12