Enhance/move data to content files #269
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
diwanshuster
added
In Progress
arunshenoy99
removed
the
In Progress
There was a problem hiding this comment.
Looks good @diwanshuster, I can think of 3 things and some minor code changes that will help improve the quality of our codebase.
- We can delete
src/OnboardingSPA/components/StepOverview/andsrc/OnboardingSPA/pages/Steps/DesignThemes/for now and completely remove thedescriptionkey from all routes? - Let's duplicate the
titlevalue forheadingkeys in steps where it does not exist, and copy thisheadingto the respectivecontents.jsfile? - Rename the
headingkey in our step data totooltipTextsince that is the only context in which it is being used.
Comment on lines
8
to
15
| subheading: __( | ||
| "We'll paint everything with your colors for a fresh, crisp look.", | ||
| 'wp-module-onboarding' | ||
| ), | ||
| description: __( | ||
| 'Strong contrast and clear readability help your words jump off the screen.', | ||
| 'wp-module-onboarding' | ||
| ), |
There was a problem hiding this comment.
I believe this is not being used anywhere.
Comment on lines
8
to
15
| subheading: __( | ||
| 'Your site header helps organize your story for visitors.', | ||
| 'wp-module-onboarding' | ||
| ), | ||
| description: __( | ||
| 'A well-organized site makes visitors feel smart, helping you keep and convert them.', | ||
| 'wp-module-onboarding' | ||
| ), |
There was a problem hiding this comment.
This is also not being used anywhere.
| @@ -134,7 +135,7 @@ const StepDesignHomepageMenu = () => { | |||
| <div className="homepage_preview"> | |||
| <HeadingWithSubHeading | |||
| title={ currentStep?.heading } | |||
There was a problem hiding this comment.
Let's bring over this heading to contents.js?
| 'Pick a starter layout you can refine and remix with your content', | ||
| 'wp-module-onboarding' | ||
| ), | ||
| description: __( |
There was a problem hiding this comment.
The description not being used anywhere.
Comment on lines
8
to
15
| subheading: __( | ||
| 'Impress your visitors with strong branding and aesthetics.', | ||
| 'wp-module-onboarding' | ||
| ), | ||
| description: __( | ||
| "Good typography uses style and proportions to give your words identity and priority. What's your story? Your focus?", | ||
| 'wp-module-onboarding' | ||
| ), |
There was a problem hiding this comment.
This is not being used anywhere. Good to clean up.
| @@ -7,6 +7,10 @@ const getContents = () => { | |||
| 'Do you want to enable tax rates and calculations?', | |||
| 'wp-module-onboarding' | |||
| ), | |||
| description: __( | |||
There was a problem hiding this comment.
This is not being used anywhere. Good to clean up.
| @@ -86,12 +85,14 @@ const StepSiteFeatures = () => { | |||
| getCustomPlugins(); | |||
| }, [] ); | |||
|
|
|||
| const content = getContents(); | |||
|
|
|||
| return ( | |||
| <CommonLayout> | |||
| <div style={ { margin: '100px' } }> | |||
| <HeadingWithSubHeading | |||
| title={ currentStep?.heading } | |||
There was a problem hiding this comment.
Let's move the heading to contents.js?
| 'Our toolbox of Plugins & Services is your toolbox.', | ||
| 'wp-module-onboarding' | ||
| ), | ||
| description: __( |
There was a problem hiding this comment.
This is not being used anywhere. Good to clean up.
| @@ -147,14 +148,16 @@ const StepSitePages = () => { | |||
| if ( themeStatus === THEME_STATUS_ACTIVE ) getSitePages(); | |||
| }, [ themeStatus ] ); | |||
|
|
|||
| const content = getContents(); | |||
|
|
|||
| return ( | |||
| <DesignStateHandler> | |||
| <GlobalStylesProvider> | |||
| <CommonLayout> | |||
| <div className="site-pages"> | |||
| <HeadingWithSubHeading | |||
| title={ currentStep?.heading } | |||
| 'Begin closer to the finish line than a blank canvas.', | ||
| 'wp-module-onboarding' | ||
| ), | ||
| description: __( |
There was a problem hiding this comment.
This is not being used anywhere. Good to clean up.
added 9 commits
July 18, 2023 12:00
arunshenoy99
approved these changes
Jul 21, 2023
arunshenoy99
added
QA
officiallygod
added
Ready to merge
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.