Skip to content

Commit

Permalink
Merge pull request #6596 from ampproject/feature/6071-onboarding-site…
Browse files Browse the repository at this point in the history
…-review

Introduce Site Review to the Onboarding Wizard
  • Loading branch information
westonruter authored Sep 29, 2021
2 parents 7ae8cfb + ce6c456 commit ed75d8b
Show file tree
Hide file tree
Showing 66 changed files with 1,655 additions and 1,002 deletions.
1 change: 1 addition & 0 deletions .phpstorm.meta.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
'admin.paired_browsing' => \AmpProject\AmpWP\Admin\PairedBrowsing::class,
'admin.plugin_row_meta' => \AmpProject\AmpWP\Admin\PluginRowMeta::class,
'admin.polyfills' => \AmpProject\AmpWP\Admin\Polyfills::class,
'admin.user_rest_endpoint_extension' => \AmpProject\AmpWP\Admin\UserRESTEndpointExtension::class,
'admin.validation_counts' => \AmpProject\AmpWP\Admin\ValidationCounts::class,
'amp_slug_customization_watcher' => \AmpProject\AmpWP\AmpSlugCustomizationWatcher::class,
'background_task_deactivator' => \AmpProject\AmpWP\BackgroundTask\BackgroundTaskDeactivator::class,
Expand Down
19 changes: 17 additions & 2 deletions assets/src/components/amp-setting-toggle/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* External dependencies
*/
import PropTypes from 'prop-types';
import classnames from 'classnames';

/**
* WordPress dependencies
Expand All @@ -19,14 +20,27 @@ import './style.css';
*
* @param {Object} props Component props.
* @param {boolean} props.checked Whether the toggle is on.
* @param {boolean} props.compact Whether the toggle is compact/small.
* @param {boolean} props.disabled Whether the toggle is disabled.
* @param {Function} props.onChange Change handler.
* @param {string} props.text Toggle text.
* @param {Object|string} props.title Toggle title.
*/
export function AMPSettingToggle( { checked, disabled = false, onChange, text, title } ) {
export function AMPSettingToggle( {
checked,
compact = false,
disabled = false,
onChange,
text,
title,
} ) {
return (
<div className={ `amp-setting-toggle ${ disabled ? 'amp-setting-toggle--disabled' : '' }` }>
<div
className={ classnames( 'amp-setting-toggle', {
'amp-setting-toggle--disabled': disabled,
'amp-setting-toggle--compact': compact,
} ) }
>
<ToggleControl
checked={ ! disabled && checked }
label={ (
Expand Down Expand Up @@ -55,6 +69,7 @@ export function AMPSettingToggle( { checked, disabled = false, onChange, text, t
}
AMPSettingToggle.propTypes = {
checked: PropTypes.bool.isRequired,
compact: PropTypes.bool,
disabled: PropTypes.bool,
onChange: PropTypes.func.isRequired,
text: PropTypes.string,
Expand Down
11 changes: 11 additions & 0 deletions assets/src/components/amp-setting-toggle/style.css
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,14 @@
.amp-setting-toggle--disabled .components-toggle-control__label {
pointer-events: none;
}

.amp .amp-setting-toggle--compact .components-form-toggle {

@media (min-width: 783px) {
margin-right: 1rem;
}
}

.amp .amp-setting-toggle--compact .amp-setting-toggle__label-text p {
margin: 0;
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

54 changes: 54 additions & 0 deletions assets/src/components/nav-menu/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/**
* External dependencies
*/
import PropTypes from 'prop-types';
import classnames from 'classnames';

/**
* Internal dependencies
*/
import './style.scss';
import { Selectable } from '../selectable';

/**
* Nav menu.
*
* @param {Object} props Component props.
* @param {Array} props.links List of links.
* @param {Function} props.onClick Click handler that receives the original event and a clicked link object.
*/
export function NavMenu( { links = [], onClick } ) {
return (
<Selectable
ElementName="nav"
className="nav-menu"
>
<ul className="nav-menu__list">
{ links.map( ( link ) => (
<li key={ link.url } className="nav-menu__item">
<a
className={ classnames( 'nav-menu__link', {
'nav-menu__link--active': link.isActive,
} ) }
href={ link.url }
onClick={ ( event ) => onClick( event, link ) }
>
{ link.label }
</a>
</li>
) ) }
</ul>
</Selectable>
);
}

NavMenu.propTypes = {
links: PropTypes.arrayOf(
PropTypes.shape( {
url: PropTypes.string,
label: PropTypes.string,
isActive: PropTypes.bool,
} ),
),
onClick: PropTypes.func,
};
64 changes: 64 additions & 0 deletions assets/src/components/nav-menu/style.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
.nav-menu__item {
margin: 0;

& + & {
border-top: 1px solid var(--amp-settings-color-border);
}
}

.amp .nav-menu__link {
align-items: center;
color: var(--amp-settings-color-muted);
display: flex;
flex-flow: row nowrap;
font-size: 14px;
justify-content: space-between;
padding: 0.75rem 1rem;
text-decoration: none;
transition: background-color 120ms ease;

&.nav-menu__link--active,
&:hover {
background-color: var(--very-light-gray);
color: var(--amp-settings-color-muted);
}

&:focus {
box-shadow: none;
}

&::after {
border: 2px solid;
border-bottom: none;
border-left: none;
content: "";
display: block;
flex: 0 0 auto;
height: 8px;
margin-left: 1rem;
transform: rotateZ(45deg);
width: 8px;
}
}

// Nav menu as a selectable component.
.nav-menu.selectable {
padding: 0;

.nav-menu__list {
margin: 0;
padding: 0 1rem;
}

.nav-menu__link {
margin: 0 -1rem;
}

.nav-menu__item:first-child .nav-menu__link {
border-radius: 8px 8px 0 0;
}

.nav-menu__item:last-child .nav-menu__link {
border-radius: 0 0 8px 8px;
}
}
34 changes: 34 additions & 0 deletions assets/src/components/nav-menu/test/__snapshots__/nav-menu.js.snap

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

101 changes: 101 additions & 0 deletions assets/src/components/nav-menu/test/nav-menu.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@

/**
* External dependencies
*/
import { act } from 'react-dom/test-utils';
import { create } from 'react-test-renderer';

/**
* WordPress dependencies
*/
import { render } from '@wordpress/element';

/**
* Internal dependencies
*/
import { NavMenu } from '../index';

let container;

const links = [
{
url: 'https://example.com/foo',
label: 'Foo',
isActive: false,
},
{
url: 'https://example.com/bar',
label: 'Bar',
isActive: true,
},
];

describe( 'NavMenu', () => {
beforeEach( () => {
container = document.createElement( 'div' );
document.body.appendChild( container );
} );

afterEach( () => {
document.body.removeChild( container );
container = null;
} );

it( 'matches the snapshot', () => {
const wrapper = create( <NavMenu links={ links } /> );

expect( wrapper.toJSON() ).toMatchSnapshot();
} );

it( 'renders a nav menu with a list of links', () => {
act( () => {
render(
<NavMenu links={ links } />,
container,
);
} );

expect( container.querySelector( 'nav' ) ).not.toBeNull();
expect( container.querySelector( 'ul' ) ).not.toBeNull();
expect( container.querySelectorAll( 'li' ) ).toHaveLength( 2 );
} );

it( 'contains correct links', () => {
act( () => {
render(
<NavMenu links={ links } />,
container,
);
} );

expect( container.querySelector( 'nav' ).textContent ).toBe( 'FooBar' );
expect( container.querySelectorAll( 'a' ) ).toHaveLength( 2 );
expect( container.querySelectorAll( 'a[href="https://example.com/foo"]' ) ).toHaveLength( 1 );
expect( container.querySelectorAll( 'a[href="https://example.com/bar"]' ) ).toHaveLength( 1 );
expect( container.querySelectorAll( 'a[class*="--active"]' ) ).toHaveLength( 1 );
expect( container.querySelector( 'a[class*="--active"]' ).getAttribute( 'href' ) ).toBe( 'https://example.com/bar' );
} );

it( 'calls the handler function on click', () => {
const handler = jest.fn();

act( () => {
render(
<NavMenu links={ links } onClick={ handler } />,
container,
);
} );

act(
() => {
container.querySelector( 'a' ).click();
},
);

expect( handler ).toHaveBeenCalledTimes( 1 );

const [ event, link ] = handler.mock.calls[ 0 ];
expect( event.type ).toBe( 'click' );
expect( link ).toBe( links[ 0 ] );
} );
} );
16 changes: 11 additions & 5 deletions assets/src/components/phone/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,22 @@ import PropTypes from 'prop-types';
* Internal dependencies
*/
import './style.css';
import { Loading } from '../loading';

/**
* Component resembling a phone with a screen.
*
* @param {Object} props Component props.
* @param {any} props.children The elements to display in the screen.
* @param {Object} props Component props.
* @param {any} props.children The elements to display in the screen.
* @param {boolean} props.isLoading Flag indicating if the content is loading.
*/
export function Phone( { children } ) {
export function Phone( { children, isLoading = false } ) {
return (
<div className="phone">
<div className="phone-inner">
<div className={ `phone ${ isLoading ? 'is-loading' : '' }` }>
<div className="phone__inner">
<div className="phone__overlay">
<Loading />
</div>
{ children }
</div>
</div>
Expand All @@ -26,4 +31,5 @@ export function Phone( { children } ) {

Phone.propTypes = {
children: PropTypes.any,
isLoading: PropTypes.bool,
};
Loading

0 comments on commit ed75d8b

Please sign in to comment.