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

Site Settings: abstract a site-centered no-sidebar scenario Card into a separate component #18273

Closed
wants to merge 1 commit into from

Conversation

AnnaMag
Copy link
Contributor

@AnnaMag AnnaMag commented Sep 26, 2017

Purpose:
Create a Card component, which is centered on sites without a navigation sidebar.

Context:
Throughout the Jetpack Disconnect Flow we use a site-centered Card component with varying headers and subheaders, but repeated styling. Aside from cleaning code repeats for the Disconnect Flow, IMO this set-up could be reused in the other contexts (great for UX flows).

To test:

Refactor of #18119.

To test:

For Jetpack and non-Atomic sites:

  • the Card should show /settings/disconnect-site/:site, where :site is the Jetpack site.

Remaining sites have no access and should be redirected to /settings/general/.

Visuals:
30777449-b5d262ce-a0b2-11e7-99c7-994531670b2c

@AnnaMag AnnaMag self-assigned this Sep 26, 2017
@AnnaMag AnnaMag added [Feature] Site Settings All other general site settings. [Status] In Progress labels Sep 26, 2017
@AnnaMag AnnaMag force-pushed the update/site-settings-extract-card branch from 5c3fef3 to eb749e6 Compare September 26, 2017 14:54
Throughout the Jetpack Disconnect Flow we use
a site-centered Card component without a navigation
sidebar. Headers and subheaders vary, thus are included
as props passed to the `CenteredCard` component.

The goal is to decrease code repeats and aid reusability
of such components in the future.
@AnnaMag AnnaMag requested a review from tyxla September 28, 2017 13:09
@AnnaMag AnnaMag added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Sep 28, 2017
@AnnaMag AnnaMag removed the request for review from tyxla September 28, 2017 13:20
@AnnaMag AnnaMag removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Sep 28, 2017
@ockham
Copy link
Contributor

ockham commented Oct 23, 2017

Closing -- I think this is addressed well enough by composition as per #18749 🙂

@ockham ockham closed this Oct 23, 2017
@ockham ockham deleted the update/site-settings-extract-card branch October 23, 2017 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Site Settings All other general site settings. OSS Citizen
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants