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

Use React-based component for Stylesheets box on Validated URL screen #6442

Open
wants to merge 58 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
58 commits
Select commit Hold shift + click to select a range
7c15cc9
Add `validated-urls` endpoint returning stylesheets data
delawski Jul 1, 2021
73568d9
Introduce `ValidatedUrlData` that is returned by the `ValidatedUrlDat…
delawski Jul 5, 2021
8aba364
Add Validated URL JavaScript entry point
delawski Jul 5, 2021
e43d296
Use local variable for storing current post
delawski Jul 7, 2021
22aae9e
Remove dead code
delawski Jul 7, 2021
8824f83
Fix and update unit tests
delawski Jul 7, 2021
56ef32f
Calculate stylesheets sizes and add to app context
delawski Jul 7, 2021
e71fbf1
Add `StylesheetsSummary` component
delawski Jul 7, 2021
bd11a06
Add locale-aware number formatting helper
delawski Jul 8, 2021
79a4ac7
Introduce `FormattedMemoryValue` with tests
delawski Jul 8, 2021
bf35648
Add CSS budget-related values to stylesheets summary table
delawski Jul 8, 2021
74c8732
Move icons to parent `components` directory
delawski Jul 9, 2021
7fac19b
Introduce validation status icon component
delawski Jul 9, 2021
a9e3c39
Add status icons to stylesheets summary table
delawski Jul 9, 2021
cd81f7b
Add warning and error notices related to CSS usage
delawski Jul 9, 2021
bf49de9
Use flags instead of string prop
delawski Jul 9, 2021
b6ed867
Use flags for stylesheets budget status designators
delawski Jul 9, 2021
8f81c07
Use more accurate naming convention
delawski Jul 9, 2021
e7ba442
Fix minor calculation bugs and update copy
delawski Jul 9, 2021
3503fad
Add missing/stale stylesheets errors
delawski Jul 14, 2021
cd96844
Add notice when CSS processing is limited
delawski Jul 14, 2021
3693b47
Fix `propTypes` definition
delawski Jul 14, 2021
6652eb7
Calculate stylesheets stats in separate side effect
delawski Jul 14, 2021
7bab544
Add stylesheet details table (WIP)
delawski Jul 14, 2021
546018b
Update unit test
delawski Jul 14, 2021
8db7369
Add CSS code diff component to stylesheet details view
delawski Jul 16, 2021
2fbca5d
Add missing dependency to `useMemo`
delawski Jul 21, 2021
62fd84c
Let child component test `origin` prop instead of parent
delawski Jul 21, 2021
723d95d
Migrate `AMP_Validation_Error_Taxonomy::summarize_sources` method to JS
delawski Jul 21, 2021
ede4e2e
Add `SourceLabel` component
delawski Jul 21, 2021
24ec238
Bake in business logic into `summarizeSources` function
delawski Jul 22, 2021
21d2c7a
Add sources summary column content
delawski Jul 22, 2021
08e6310
Expose validated environment data in REST API and use it in Sources
delawski Jul 23, 2021
1136aad
Use validated URL context directly in child component
delawski Jul 23, 2021
fe3e041
Use better name for validated URL context provider
delawski Jul 23, 2021
eec19a6
Move stylesheets stats logic to more suitable place ie. context provider
delawski Jul 23, 2021
9443d23
Use constants instead of flags for source types
delawski Jul 27, 2021
30a6306
Add detailed sources stack to Stylesheets meta box table
delawski Jul 28, 2021
cf9866c
Memoize summarized sources
delawski Jul 28, 2021
b9614c9
Add loading state to shaken CSS tokens diff
delawski Jul 28, 2021
e1165f6
Add some spacing to shaken tokens diff toggle
delawski Jul 28, 2021
18ffb6b
Add Validated URL data preloading via `RESTPreloader`
delawski Jul 29, 2021
937fa02
Add missing import
delawski Jul 29, 2021
4ba9b42
Move `validated-url` endpoint to separate controller and add schema
delawski Jul 29, 2021
e8087f5
Remove server-side rendered Stylesheets meta box
delawski Jul 29, 2021
78724d5
Fix phpcs issues
delawski Jul 29, 2021
658fd5a
Clean up styles for Stylesheets meta box
delawski Jul 29, 2021
6cd5968
Fix php unit test
delawski Jul 29, 2021
477831d
Add plugins context and convert slug into plugin name
delawski Jul 29, 2021
cb7d331
Add themes context and convert slug into theme name
delawski Jul 29, 2021
05361ad
Skip including Gutenberg in summary if there is another plugin
delawski Jul 30, 2021
a6708ff
Ensure property is set
delawski Aug 17, 2021
612560c
Ensure sources prop has default value
delawski Aug 17, 2021
0403e29
Use default labels for unrecognized themes and plugins
delawski Aug 17, 2021
9d0ec88
Increase test coverage
delawski Aug 17, 2021
d7581b5
Replace deprecated `assertStringContains` with `assertStringContainsS…
delawski Aug 17, 2021
b9de4d5
Apply suggestions from code review
delawski Aug 17, 2021
26525f8
Add missing service
delawski Aug 17, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .phpstorm.meta.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,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.rest_preloader' => \AmpProject\AmpWP\Admin\RESTPreloader::class,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is missing the second service that was added:

'validated_url_rest_controller' => \AmpProject\AmpWP\Validation\ValidatedUrlRESTController::class,

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I've added it back in 26525f8.

'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 Expand Up @@ -53,6 +54,7 @@
'site_health_integration' => \AmpProject\AmpWP\Admin\SiteHealth::class,
'url_validation_cron' => \AmpProject\AmpWP\Validation\URLValidationCron::class,
'url_validation_rest_controller' => \AmpProject\AmpWP\Validation\URLValidationRESTController::class,
'validated_url_rest_controller' => \AmpProject\AmpWP\Validation\ValidatedUrlRESTController::class,
'validated_url_stylesheet_gc' => \AmpProject\AmpWP\BackgroundTask\ValidatedUrlStylesheetDataGarbageCollection::class,
] )
);
Expand Down
2 changes: 0 additions & 2 deletions assets/css/src/amp-validation-error-taxonomy.scss
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
@import "./components/amp-stylesheet-list";
@import "./components/amp-stylesheet-summary";
@import "./components/details-column";
@import "./components/error-details-toggle";
@import "./components/wp-list-table-grid-lines";
Expand Down
4 changes: 1 addition & 3 deletions assets/css/src/components/_details-column.scss
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,7 @@
}
}

.details-attributes__summary,
.toggle-stylesheet-details {
.details-attributes__summary {

&::after {
background-image: url("../images/down-triangle.svg");
Expand All @@ -46,7 +45,6 @@
}

tr.expanded .details-attributes__summary,
tr.expanded .toggle-stylesheet-details,
details[open] > .details-attributes__summary {

&::after {
Expand Down
4 changes: 2 additions & 2 deletions assets/images/amp-alert.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion assets/images/amp-valid.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions assets/images/amp-warning.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
25 changes: 0 additions & 25 deletions assets/src/amp-validation/amp-validated-url-post-edit-screen.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ domReady( () => {
handleRowEvents();
handleBulkActions();
watchForUnsavedChanges();
setupStylesheetsMetabox();
handleCopyToClipboardButtons();
} );

Expand Down Expand Up @@ -382,27 +381,3 @@ const handleBulkActions = () => {
} );
} );
};

/**
* Set up stylesheet metabox.
*/
const setupStylesheetsMetabox = () => {
const metabox = document.getElementById( 'amp_stylesheets' );

for ( const toggleStylesheetDetailsButton of metabox.querySelectorAll( '.toggle-stylesheet-details' ) ) {
const row = toggleStylesheetDetailsButton.closest( 'tr' );
toggleStylesheetDetailsButton.addEventListener( 'click', () => {
row.classList.toggle( 'expanded' );
} );
}

for ( const stylesheetDetailsElements of metabox.querySelectorAll( '.stylesheet-details' ) ) {
const shakenStylesheetContainer = stylesheetDetailsElements.querySelector( '.shaken-stylesheet' );
const showRemovedStylesCheckbox = stylesheetDetailsElements.querySelector( '.show-removed-styles' );
if ( showRemovedStylesCheckbox ) {
showRemovedStylesCheckbox.addEventListener( 'click', () => {
shakenStylesheetContainer.classList.toggle( 'removed-styles-shown', showRemovedStylesCheckbox.checked );
} );
}
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { useDispatch, useSelect } from '@wordpress/data';
import AMPValidationErrorsKeptIcon from '../../../../images/amp-validation-errors-kept.svg';
import BellIcon from '../../../../images/bell-icon.svg';
import { BLOCK_VALIDATION_STORE_KEY } from '../../store';
import { StatusIcon } from '../icon';
import { StatusIcon } from '../../../components/icon';
import { SidebarNotification } from '../sidebar-notification';
import { useAMPDocumentToggle } from '../../hooks/use-amp-document-toggle';
import { useErrorsFetchingStateChanges } from '../../hooks/use-errors-fetching-state-changes';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { useDispatch, useSelect } from '@wordpress/data';
*/
import AMPValidationErrorsKeptIcon from '../../../../images/amp-validation-errors-kept.svg';
import { BLOCK_VALIDATION_STORE_KEY } from '../../store';
import { StatusIcon } from '../icon';
import { StatusIcon } from '../../../components/icon';
import { SidebarNotification } from '../sidebar-notification';
import { useErrorsFetchingStateChanges } from '../../hooks/use-errors-fetching-state-changes';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { useDispatch } from '@wordpress/data';
/**
* Internal dependencies
*/
import { ToolbarIcon } from '../icon';
import { ToolbarIcon } from '../../../components/icon';
import { PLUGIN_NAME, SIDEBAR_NAME } from '../../plugins/amp-block-validation';
import { useAMPDocumentToggle } from '../../hooks/use-amp-document-toggle';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { useSelect } from '@wordpress/data';
* Internal dependencies
*/
import { BLOCK_VALIDATION_STORE_KEY } from '../store';
import { MoreMenuIcon, ToolbarIcon } from '../components/icon';
import { MoreMenuIcon, ToolbarIcon } from '../../components/icon';
import { Sidebar } from '../components/sidebar';
import { InvalidBlockOutline } from '../components/invalid-block-outline';
import { usePostDirtyStateChanges } from '../hooks/use-post-dirty-state-changes';
Expand Down
58 changes: 58 additions & 0 deletions assets/src/components/formatted-memory-value/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/**
* WordPress dependencies
*/
import { __, _x } from '@wordpress/i18n';

/**
* External dependencies
*/
import PropTypes from 'prop-types';

/**
* Internal dependencies
*/
import { numberFormat } from '../../utils/number-format';

function getMemoryUnit( unit ) {
if ( ! unit || typeof unit !== 'string' ) {
return null;
}

switch ( unit.toLowerCase() ) {
case 'b':
return {
name: __( 'bytes', 'amp' ),
abbreviation: _x( 'B', 'abbreviation for bytes', 'amp' ),
};
case 'kb':
return {
name: __( 'kilobytes', 'amp' ),
abbreviation: _x( 'kB', 'abbreviation for kilobytes', 'amp' ),
};
default:
return null;
}
}

export default function FormattedMemoryValue( { value, unit } ) {
const memoryUnit = getMemoryUnit( unit );

return (
<>
{ numberFormat( value ) }
{ memoryUnit && (
<>
{ ' ' }
<abbr title={ memoryUnit.name }>
{ memoryUnit.abbreviation }
</abbr>
</>
) }
{ ! memoryUnit && unit && ` ${ unit }` }
</>
);
}
FormattedMemoryValue.propTypes = {
value: PropTypes.oneOfType( [ PropTypes.number, PropTypes.string ] ).isRequired,
unit: PropTypes.string,
};
95 changes: 95 additions & 0 deletions assets/src/components/formatted-memory-value/test/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
/**
* WordPress dependencies
*/
import { render } from '@wordpress/element';

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

/**
* Internal dependencies
*/
import FormattedMemoryValue from '..';

let container;

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

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

it( 'prints bare value if no unit is provided', () => {
act( () => {
render(
<FormattedMemoryValue value={ 123 } />,
container,
);
} );

expect( container.textContent ).toBe( '123' );
} );

it( 'prints correct output if value is a string', () => {
act( () => {
render(
<FormattedMemoryValue value="234" />,
container,
);
} );

expect( container.textContent ).toBe( '234' );
} );

it( 'prints correct output for unknown units', () => {
act( () => {
render(
<FormattedMemoryValue value="234" unit="FooBar" />,
container,
);
} );

expect( container.textContent ).toBe( '234 FooBar' );
} );

it( 'prints correct value and unit', () => {
act( () => {
render(
<FormattedMemoryValue value="345" unit="B" />,
container,
);
} );

expect( container.textContent ).toBe( '345 B' );
expect( container.querySelector( 'abbr' ) ).not.toBeNull();
expect( container.querySelector( 'abbr' ).title ).toBe( 'bytes' );
} );

it.each( [
[ 'bytes', 'B', 'B' ],
[ 'bytes', 'b', 'B' ],
[ 'kilobytes', 'kB', 'kB' ],
[ 'kilobytes', 'kb', 'kB' ],
[ 'kilobytes', 'KB', 'kB' ],
] )(
'prints correct %s value and unit for the following input unit: %s',
( fullName, inputUnit, abbreviation ) => {
act( () => {
render(
<FormattedMemoryValue value="100" unit={ inputUnit } />,
container,
);
} );

expect( container.querySelector( 'abbr' ).title ).toBe( fullName );
expect( container.querySelector( 'abbr' ).textContent ).toBe( abbreviation );
},
);
} );
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,12 @@ import PropTypes from 'prop-types';
* Internal dependencies
*/
import './style.css';
import AMPLogoIcon from '../../../../images/amp-logo-icon.svg';
import AMPToolbarIcon from '../../../../images/amp-icon-toolbar.svg';
import AMPToolbarIconBroken from '../../../../images/amp-toolbar-icon-broken.svg';
import AMPLogoIcon from '../../../images/amp-logo-icon.svg';
import AMPToolbarIcon from '../../../images/amp-icon-toolbar.svg';
import AMPToolbarIconBroken from '../../../images/amp-toolbar-icon-broken.svg';
import AMPValidIcon from '../../../images/amp-valid.svg';
import AMPWarningIcon from '../../../images/amp-warning.svg';
import AMPAlertIcon from '../../../images/amp-alert.svg';

/**
* Plugin icon.
Expand Down Expand Up @@ -94,3 +97,43 @@ export function StatusIcon( { broken = false } ) {
StatusIcon.propTypes = {
broken: PropTypes.bool,
};

/**
* Renders the validation status icon.
*
* @param {Object} props
* @param {boolean} props.isError Flag indicating the icon is for an error status.
* @param {boolean} props.isWarning Flag indicating the icon is for a warning status.
* @param {boolean} props.isValid Flag indicating the icon is for a valid status.
* @param {boolean} props.isBoxed Whether the icon should be contained in a box.
*/
export function ValidationStatusIcon( { isError, isWarning, isValid, isBoxed = false, ...rest } ) {
let type;

if ( isError ) {
type = 'error';
} else if ( isWarning ) {
type = 'warning';
} else if ( isValid ) {
type = 'valid';
} else {
return null;
}

return (
<span
className={ `amp-validation-status-icon amp-validation-status-icon--${ type } ${ isBoxed ? 'amp-validation-status-icon--boxed' : '' }` }
{ ...rest }
>
{ isValid && <AMPValidIcon /> }
{ isWarning && <AMPWarningIcon /> }
{ isError && <AMPAlertIcon /> }
</span>
);
}
ValidationStatusIcon.propTypes = {
isError: PropTypes.bool,
isWarning: PropTypes.bool,
isValid: PropTypes.bool,
isBoxed: PropTypes.bool,
};
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,30 @@
.is-pressed .amp-error-count-badge {
border-color: #1e1e1e;
}

/*
* AMP validation status icons.
*/
.amp-validation-status-icon {
display: inline-block;
height: 16px;
vertical-align: middle;
width: 16px;
}

.amp-validation-status-icon--boxed {
border-radius: 5px;
padding: 2px;
}

.amp-validation-status-icon--boxed.amp-validation-status-icon--valid {
background-color: #d3e5e5;
}

.amp-validation-status-icon--boxed.amp-validation-status-icon--warning {
background-color: #fff6dd;
}

.amp-validation-status-icon--boxed.amp-validation-status-icon--error {
background-color: #f7ded4;
}
Loading