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

Focused Launch: replace custom code with <Title> and <Subtitle> components from @automattic/onboarding #47497

Closed
ciampo opened this issue Nov 17, 2020 · 4 comments · Fixed by #47615
Assignees
Labels
Good First Issue Small, contained issues. [Type] Task

Comments

@ciampo
Copy link
Contributor

ciampo commented Nov 17, 2020

Once #47418 and #47496 are implemented, code in the Focused Launch screens should be refactored to re-use <Title> and <Subtitle> components as much as possible.

Particular attention should be applied to make sure that:

  • the document outline is correct (i.e. since the modal already comes with a <h1> tag, all titles/subtitles in the Focused Launch views should start from <h2>)
  • the classnames added from the Focused Launch point of view are applied and merged correctly
@razvanpapadopol
Copy link

razvanpapadopol commented Nov 18, 2020

There's probably a need to do something similar to ActionButtons as well. For example, we could use BackButton with a different color in DomainsDetails view and PlansDetails view.

@ciampo
Copy link
Contributor Author

ciampo commented Nov 18, 2020

There's probably a need to do something similar to ActionButtons as well. For example, we could use BackButton with a different color here: https://github.com/Automattic/wp-calypso/pull/47318/files#diff-a88807ae898bfacf7689a16c723644ee8d5ddf40677fd0ce119761a404d0b897R46-R50

Agreed. We should probably work towards standardizing all components in @automattic/onboarding

@StefanNieuwenhuis
Copy link
Contributor

Screenshot:
Screenshot 2020-11-19 at 13 21 16

Code
Screenshot 2020-11-19 at 13 27 00

Do we want to turn these into <Title/> & <SubTitle/> as well?

The code is found here: packages/plans-grid/src/plans-accordion-item/index.tsx on l93-l94.

@ciampo
Copy link
Contributor Author

ciampo commented Nov 19, 2020

Do we want to turn these into <Title/> & as well?

The plan names in the plans grid have different typography styles from the onboarding-specific title/subtitle.

Let's only focus on updating title/subtitle in the Focused Launch views, so that

  • all views use <Title> and <Subtitle>
  • all instances of <Title> and <Subtitle> in Focused Launch use the correct HTML tags
  • the required additional styles are applied correctly after merging classnames

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good First Issue Small, contained issues. [Type] Task
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants