refactor: NextStep and NextStepAlt
requested to merge 3734-refactor-how-next-steps-components-are-used-in-contentful-and-buyer-experience into main
Step 1: What is changing in this MR?
TLDR: use new entryID
prop to control content, remove variant
prop, some layout booleans
- remove support for
variant
property inNextStep
andNextStepAlt
- spread out any existing layout properties out as dummy variable booleans:
isSingleColumn
andhasVerticalLine
. Both of these do exactly what it sounds like they do, they essentially update the CSS of the component to match some given design specs: slack message about that. I need to give more thought into this, but I am at a baseline trying to cover existing variants found in BE. I believe that is a good place to start. - introduces
isLikeAltLayout
, a computed property that checks for the existence of calculator variant properties. If they exist, the layout of the right column will look likeNextStepAlt
- spread out any existing layout properties out as dummy variable booleans:
- Add support for
entryID
property, which is the entry for the Next Steps content type instance coming from Contentful. This controls the actual copy and this is coming from Contentful. If one is not supplied, it will default to the "main" one, which is used in 99% of pages.- As an extension of my previous comment, it is my opinion that we should not allow content editors to edit these CSS/layout properties from Contentful. As the NextStep component has relatively few variants for the number of pages we have, and given that the variants are mostly all coming from one-off pages, I believe this to be premature optimization and to add more complexity with little value gain.
- Note that this MR supports content coming from Contentful. If want to use this component in
content
YAML, we will need to provide a prop calledcontentData
that follows the same data interface as the data coming from contentful:CtfEntry<CtfNextStep>
. - This implementation should not break any localization efforts coming from Contentful. These components should be updated using field-level localization for text and entry-level localization for buttons/other entries
- I removed the
locale
prop inNextStepAlt
and have it set up as a computed property instead
TODO
- I have to do more testing on the new props to make sure someone can't break a layout by getting multiple
true
values forisSingleColumn
,hasVerticalLine
, andisLikeAltLayout
AFTER APPROVAL BEFORE MERGING
- This entry has changed and will need to be published as this MR best merged
- Remove the variant field coming from the Next Steps Content type.
- publish: https://app.contentful.com/spaces/xz1dnu24egyd/entries/5DAr4T0ZSMTWg1YOJWaCuV?focusedField=internalName
FURTHER ITERATION(?)
- I think the calculator variant JSON should be merged into the BlockGroup field in contentful.
Production | Review app |
---|---|
https://about.gitlab.com/ | https://3734-refactor-how-next-steps-components-are-used-in-contentful.about.gitlab-review.app/ |
Affected pages: Really all pages, but the ones that are important that we expect to stay the same
Home, Partners, Solutions
Step 2: Ensure that your changes comply with the following, where applicable:
-
I, the Assignee, have run Axe tools on any updated pages, and fixed the relevant accessibility issues. -
These changes work on both Safari, Chrome, and Firefox. -
These changes have been reviewed for Visual Quality Assurance and Functional Quality Assurance on Mobile, Desktop, and Tablet. -
These changes work with our Google Analytics and SEO tools. -
These changes have been documented as expected.
Build Variables:
-
Use Contentful Preview API
Closes #3734 (closed)
Edited by Javier Garcia