From aeb8bf9964d4e6eaa233a0fe3ad96bf7959d077f Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sun, 23 Sep 2018 20:57:58 -0700 Subject: [PATCH 1/6] Delete dead code: amp-validation-error-detail-toggle.js --- .../src/amp-validation-error-detail-toggle.js | 89 ------------------- 1 file changed, 89 deletions(-) delete mode 100644 assets/src/amp-validation-error-detail-toggle.js diff --git a/assets/src/amp-validation-error-detail-toggle.js b/assets/src/amp-validation-error-detail-toggle.js deleted file mode 100644 index d5ad388fbf8..00000000000 --- a/assets/src/amp-validation-error-detail-toggle.js +++ /dev/null @@ -1,89 +0,0 @@ -/** - * WordPress dependencies - */ -import domReady from '@wordpress/dom-ready'; - -/** - * Localized data - */ -import { btnAriaLabel } from 'amp-validation-i18n'; - -const OPEN_CLASS = 'is-open'; - -/** - * Adds detail toggle buttons to the header and footer rows of the validation error "details" column. - * The buttons are added via JS because there's no easy way to append them to the heading of a sortable - * table column via backend code. - */ -function addToggleButtons() { - const addButtons = ( th ) => { - const span = document.createElement( 'span' ); - span.classList.add( 'toggle-button-flex-container' ); - while ( th.firstChild ) { - span.appendChild( th.removeChild( th.firstChild ) ); - } - - const button = document.createElement( 'button' ); - button.setAttribute( 'aria-label', btnAriaLabel ); - button.setAttribute( 'type', 'button' ); - button.setAttribute( 'class', 'error-details-toggle' ); - span.appendChild( button ); - th.appendChild( span ); - }; - - [ ...document.querySelectorAll( 'th.column-details.manage-column' ) ].forEach( addButtons ); - [ ...document.querySelectorAll( 'th.manage-column.column-sources_with_invalid_output' ) ].forEach( addButtons ); -} - -/** - * Adds a listener toggling all details in the error type taxonomy details column. - */ -function addToggleListener() { - let open = false; - - const details = [ ...document.querySelectorAll( '.column-details details, .column-sources_with_invalid_output details' ) ]; - const toggleButtons = [ ...document.querySelectorAll( 'button.error-details-toggle' ) ]; - const onButtonClick = () => { - open = ! open; - toggleButtons.forEach( btn => { - btn.classList.toggle( OPEN_CLASS ); - } ); - details.forEach( detail => { - if ( open ) { - detail.setAttribute( 'open', true ); - } else { - detail.removeAttribute( 'open' ); - } - } ); - }; - - window.addEventListener( 'click', event => { - if ( toggleButtons.includes( event.target ) ) { - onButtonClick(); - } - } ); -} - -/** - * Adds classes to the rows for the amp_validation_error term list table. - * - * This is needed because \WP_Terms_List_Table::single_row() does not allow for additional - * attributes to be added to the element. - */ -function addTermListTableRowClasses() { - const rows = [ ...document.querySelectorAll( '#the-list > tr' ) ]; - rows.forEach( ( row ) => { - const statusText = row.querySelector( '.column-status > .status-text' ); - if ( statusText ) { - row.classList.toggle( 'new', statusText.classList.contains( 'new' ) ); - row.classList.toggle( 'accepted', statusText.classList.contains( 'accepted' ) ); - row.classList.toggle( 'rejected', statusText.classList.contains( 'rejected' ) ); - } - } ); -} - -domReady( () => { - addToggleButtons(); - addToggleListener(); - addTermListTableRowClasses(); -} ); From 7a944a9f6d74952b61f03ce4fa6755129d7f3bb7 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sun, 23 Sep 2018 20:58:59 -0700 Subject: [PATCH 2/6] Add row styling from unmoderated comments to apply to new accepted/rejected validation errors --- assets/src/amp-validation-detail-toggle.js | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/assets/src/amp-validation-detail-toggle.js b/assets/src/amp-validation-detail-toggle.js index 2ce496b6608..ab5028b9a2d 100644 --- a/assets/src/amp-validation-detail-toggle.js +++ b/assets/src/amp-validation-detail-toggle.js @@ -14,7 +14,7 @@ const OPEN_CLASS = 'is-open'; * Adds detail toggle buttons to the header and footer rows of the validation error "details" column. * The buttons are added via JS because there's no easy way to append them to the heading of a sortable * table column via backend code. - * + * * @param {string} containerSelector Selector for elements that will have the button added. * @param {string} ariaLabel Screen reader label for the button. * @return {Array} Array of added buttons. @@ -61,6 +61,24 @@ function addToggleAllListener( { btn, toggleAllButtonSelector = null, targetDeta btn.addEventListener( 'click', onButtonClick ); } +/** + * Adds classes to the rows for the amp_validation_error term list table. + * + * This is needed because \WP_Terms_List_Table::single_row() does not allow for additional + * attributes to be added to the element. + */ +function addTermListTableRowClasses() { + const rows = [ ...document.querySelectorAll( '#the-list tr' ) ]; + rows.forEach( ( row ) => { + const statusText = row.querySelector( '.column-status > .status-text' ); + if ( statusText ) { + row.classList.toggle( 'new', statusText.classList.contains( 'new' ) ); + row.classList.toggle( 'accepted', statusText.classList.contains( 'accepted' ) ); + row.classList.toggle( 'rejected', statusText.classList.contains( 'rejected' ) ); + } + } ); +} + domReady( () => { addToggleButtons( 'th.column-details.manage-column', detailToggleBtnAriaLabel ) .forEach( ( btn ) => { @@ -79,4 +97,6 @@ domReady( () => { targetDetailsSelector: 'details.source' } ); } ); + + addTermListTableRowClasses(); } ); From 95228ec55882721f7b2f08443eab0f400f82d130 Mon Sep 17 00:00:00 2001 From: John Watkins Date: Mon, 24 Sep 2018 11:17:15 -0500 Subject: [PATCH 3/6] Apply unmoderated comment styles to New Accepted/Rejected errors in Single URL view --- assets/css/amp-validation-error-taxonomy.css | 8 ++++--- assets/js/amp-invalid-url-post-edit-screen.js | 24 ++++++++++++++++--- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/assets/css/amp-validation-error-taxonomy.css b/assets/css/amp-validation-error-taxonomy.css index cc6fa294636..4ebd623de4e 100644 --- a/assets/css/amp-validation-error-taxonomy.css +++ b/assets/css/amp-validation-error-taxonomy.css @@ -52,7 +52,7 @@ details[open] .details-attributes__summary { content: ""; } -details[open] .details-attributes__summary::after, +tr.expanded .details-attributes__summary::after, details.single-error-detail[open] .single-error-detail-summary::after { transform: rotate(180deg); } @@ -185,11 +185,13 @@ details.single-error-detail[open] .single-error-detail-summary::after { } body.taxonomy-amp_validation_error .wp-list-table .new th, -body.taxonomy-amp_validation_error .wp-list-table .new td { +body.taxonomy-amp_validation_error .wp-list-table .new td, +tr.expanded.new + tr > td:first-of-type { background-color: #fef7f1; } -body.taxonomy-amp_validation_error .wp-list-table .new th.check-column { +body.taxonomy-amp_validation_error .wp-list-table .new th.check-column, +tr.expanded.new + tr > td:first-of-type { border-left: 4px solid #d54e21; } diff --git a/assets/js/amp-invalid-url-post-edit-screen.js b/assets/js/amp-invalid-url-post-edit-screen.js index 5d5c86c1eec..735ca05b9fb 100644 --- a/assets/js/amp-invalid-url-post-edit-screen.js +++ b/assets/js/amp-invalid-url-post-edit-screen.js @@ -287,14 +287,32 @@ var ampInvalidUrlPostEditScreen = ( function() { // eslint-disable-line no-unuse * And sets this as the src of the status icon . */ component.handleStatusChange = function handleStatusChange() { - const onChange = function( event ) { + const setRowStatusClass = function( { row, select } ) { + const statusText = select.options[ select.selectedIndex ].innerText.trim(); + + if ( statusText ) { + row.classList.toggle( 'new', 'New Rejected' === statusText || 'New Accepted' === statusText ); + row.classList.toggle( 'accepted', 'Accepted' === statusText ); + row.classList.toggle( 'rejected', 'Rejected' === statusText ); + } + }; + + const onChange = function( { event, row, select } ) { if ( event.target.matches( 'select' ) ) { component.updateSelectIcon( event.target ); + setRowStatusClass( { row, select } ); } }; - document.querySelectorAll( '.amp-validation-error-status' ).forEach( function( element ) { - element.addEventListener( 'change', onChange ); + document.querySelectorAll( 'tr[id^="tag-"]' ).forEach( function( row ) { + const select = row.querySelector( '.amp-validation-error-status' ); + + if ( select ) { + setRowStatusClass( { row, select } ); + select.addEventListener( 'change', function( event ) { + onChange( { event, row, select } ); + } ); + } } ); }; From 70c1977ee14e07ff9296b7879c325985a31ac16c Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Tue, 25 Sep 2018 17:12:15 -0300 Subject: [PATCH 4/6] Check value instead of innerText to set row classes --- assets/js/amp-invalid-url-post-edit-screen.js | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/assets/js/amp-invalid-url-post-edit-screen.js b/assets/js/amp-invalid-url-post-edit-screen.js index 735ca05b9fb..bd8f7db708c 100644 --- a/assets/js/amp-invalid-url-post-edit-screen.js +++ b/assets/js/amp-invalid-url-post-edit-screen.js @@ -1,9 +1,7 @@ /* exported ampInvalidUrlPostEditScreen */ -var ampInvalidUrlPostEditScreen = ( function() { // eslint-disable-line no-unused-vars - var component; - - component = { +const ampInvalidUrlPostEditScreen = ( function() { // eslint-disable-line no-unused-vars + let component = { data: { l10n: { unsaved_changes: '', @@ -288,13 +286,13 @@ var ampInvalidUrlPostEditScreen = ( function() { // eslint-disable-line no-unuse */ component.handleStatusChange = function handleStatusChange() { const setRowStatusClass = function( { row, select } ) { - const statusText = select.options[ select.selectedIndex ].innerText.trim(); + const acceptedValue = 3, + rejectedValue = 2; + let status = parseInt( select.options[ select.selectedIndex ].value ); - if ( statusText ) { - row.classList.toggle( 'new', 'New Rejected' === statusText || 'New Accepted' === statusText ); - row.classList.toggle( 'accepted', 'Accepted' === statusText ); - row.classList.toggle( 'rejected', 'Rejected' === statusText ); - } + row.classList.toggle( 'new', isNaN( status ) ); + row.classList.toggle( 'accepted', acceptedValue === status ); + row.classList.toggle( 'rejected', rejectedValue === status ); }; const onChange = function( { event, row, select } ) { From 13e9affaa15b0d79812bb81807e1cdc5bfd5b661 Mon Sep 17 00:00:00 2001 From: Jacob Schweitzer Date: Tue, 25 Sep 2018 19:04:34 -0300 Subject: [PATCH 5/6] Style new rows on invalid URLs index page --- assets/css/amp-validation-error-taxonomy.css | 7 ++-- assets/js/amp-invalid-urls-index.js | 36 +++++++++++++++++++ .../class-amp-invalid-url-post-type.php | 15 ++++++++ 3 files changed, 56 insertions(+), 2 deletions(-) create mode 100644 assets/js/amp-invalid-urls-index.js diff --git a/assets/css/amp-validation-error-taxonomy.css b/assets/css/amp-validation-error-taxonomy.css index 4ebd623de4e..3ac05a9613b 100644 --- a/assets/css/amp-validation-error-taxonomy.css +++ b/assets/css/amp-validation-error-taxonomy.css @@ -186,12 +186,15 @@ details.single-error-detail[open] .single-error-detail-summary::after { body.taxonomy-amp_validation_error .wp-list-table .new th, body.taxonomy-amp_validation_error .wp-list-table .new td, -tr.expanded.new + tr > td:first-of-type { +tr.expanded.new + tr > td:first-of-type, +body.post-type-amp_invalid_url .wp-list-table .new th, +body.post-type-amp_invalid_url .wp-list-table .new td { background-color: #fef7f1; } body.taxonomy-amp_validation_error .wp-list-table .new th.check-column, -tr.expanded.new + tr > td:first-of-type { +tr.expanded.new + tr > td:first-of-type, +body.post-type-amp_invalid_url .wp-list-table .new th.check-column { border-left: 4px solid #d54e21; } diff --git a/assets/js/amp-invalid-urls-index.js b/assets/js/amp-invalid-urls-index.js new file mode 100644 index 00000000000..6bee92f9187 --- /dev/null +++ b/assets/js/amp-invalid-urls-index.js @@ -0,0 +1,36 @@ +/* exported ampInvalidUrlsIndex */ + +const ampInvalidUrlsIndex = ( function() { // eslint-disable-line no-unused-vars + let component = { + classes: {} + }; + + /** + * The class for the new status + * + * @type {string} + */ + component.classes.new = 'new'; + + /** + * Boot. + */ + component.boot = function boot() { + component.highlightRowsWithNewStatus(); + }; + + /** + * Highlight rows with new status. + */ + component.highlightRowsWithNewStatus = function highlightRowsWithNewStatus() { + document.querySelectorAll( 'tr[id^="post-"]' ).forEach( function( row ) { + let newStatus = row.querySelector( 'span.status-text.' + component.classes.new ); + + if ( newStatus ) { + row.classList.toggle( 'new' ); + } + } ); + }; + + return component; +}() ); diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index b5dc3a0a4f5..fb33bb01cdd 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -209,6 +209,21 @@ public static function add_admin_hooks() { public static function enqueue_post_list_screen_scripts() { $screen = get_current_screen(); + if ( 'edit-amp_invalid_url' === $screen->id && self::POST_TYPE_SLUG === $screen->post_type ) { + wp_enqueue_script( + 'amp-invalid-urls-index', + amp_get_asset_url( 'js/amp-invalid-urls-index.js' ), + array(), + AMP__VERSION, + true + ); + wp_add_inline_script( + 'amp-invalid-urls-index', + sprintf( 'document.addEventListener( "DOMContentLoaded", function() { ampInvalidUrlsIndex.boot(); } );' ), + 'after' + ); + } + // Enqueue this on both the 'Invalid URLs' page and the single URL page. if ( 'edit-amp_invalid_url' === $screen->id || self::POST_TYPE_SLUG === $screen->id ) { wp_enqueue_style( From 0ce4f404644fb9994b3c382fad4bdfab40e96ec9 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 25 Sep 2018 20:43:38 -0700 Subject: [PATCH 6/6] Improve highlightRowsWithNewStatus and handleStatusChange --- assets/js/amp-invalid-url-post-edit-screen.js | 6 +++--- assets/js/amp-invalid-urls-index.js | 6 ++---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/assets/js/amp-invalid-url-post-edit-screen.js b/assets/js/amp-invalid-url-post-edit-screen.js index bd8f7db708c..4a56afeb0a1 100644 --- a/assets/js/amp-invalid-url-post-edit-screen.js +++ b/assets/js/amp-invalid-url-post-edit-screen.js @@ -286,9 +286,9 @@ const ampInvalidUrlPostEditScreen = ( function() { // eslint-disable-line no-unu */ component.handleStatusChange = function handleStatusChange() { const setRowStatusClass = function( { row, select } ) { - const acceptedValue = 3, - rejectedValue = 2; - let status = parseInt( select.options[ select.selectedIndex ].value ); + const acceptedValue = 3; + const rejectedValue = 2; + const status = parseInt( select.options[ select.selectedIndex ].value ); row.classList.toggle( 'new', isNaN( status ) ); row.classList.toggle( 'accepted', acceptedValue === status ); diff --git a/assets/js/amp-invalid-urls-index.js b/assets/js/amp-invalid-urls-index.js index 6bee92f9187..49ff6b7d887 100644 --- a/assets/js/amp-invalid-urls-index.js +++ b/assets/js/amp-invalid-urls-index.js @@ -24,10 +24,8 @@ const ampInvalidUrlsIndex = ( function() { // eslint-disable-line no-unused-vars */ component.highlightRowsWithNewStatus = function highlightRowsWithNewStatus() { document.querySelectorAll( 'tr[id^="post-"]' ).forEach( function( row ) { - let newStatus = row.querySelector( 'span.status-text.' + component.classes.new ); - - if ( newStatus ) { - row.classList.toggle( 'new' ); + if ( row.querySelector( 'span.status-text.' + component.classes.new ) ) { + row.classList.add( 'new' ); } } ); };