-
Notifications
You must be signed in to change notification settings - Fork 385
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
Improve URL Listing view #1397
Merged
+387
−139
Merged
Improve URL Listing view #1397
Changes from 56 commits
Commits
Show all changes
63 commits
Select commit
Hold shift + click to select a range
0131c71
Start with listing view design improvements.
miina 1e2dccf
Merge remote-tracking branch 'origin/develop' into 1362-improved_url_…
miina fd51be5
Add icons. Add tooltip skeleton. Fix sources.
miina 8ab7166
Conform to validation error data model.
kienstra b31d123
Merge in develop, resolve conflict.
kienstra 99a35b8
Update status names to latest design
kienstra e263b55
Use dashicons for tooltip icon
jacobschweitzer 9e99f1a
Add admin-tables.css file
jacobschweitzer 273cccf
Hide tooltip
jacobschweitzer df68ed8
Target class instead of id and add color to icon
jacobschweitzer 54ae458
Change Plug-In to Plugin
jacobschweitzer 4075cb0
Fix variable typo
jacobschweitzer 9094366
Add plugin icon with color from design
jacobschweitzer 604c4b9
Add theme icon with color
jacobschweitzer 90b4920
Remove extra linebreaks
jacobschweitzer 0be3a56
Add containers around sources for styling
jacobschweitzer 1cb3da7
Add margins around sources containers
jacobschweitzer 49ca0d8
Add color to WP icon
jacobschweitzer 6d7f35b
Change wording - found to removed
jacobschweitzer f6a0878
Add icons to status column
jacobschweitzer c71addb
Add amp logo icon class
jacobschweitzer ca2e7d2
Add error-status class
jacobschweitzer 36564fd
Style error statuses
jacobschweitzer 736b2f2
Add closing span tag
jacobschweitzer 7af56d6
Add color to removed elements column text
jacobschweitzer 25ab2b3
Add class to warning icons for styling
jacobschweitzer 139883e
Add color to identified and rejected icons
jacobschweitzer 2583e44
Fix translation functions, switch to esc_html__
jacobschweitzer 8649b91
Add plugins icon when there is multiple plugins
jacobschweitzer 53021cc
Hide invalid sources if more than 1 and add icon
jacobschweitzer e87cca2
Only enqueue stylesheet on its post type screens
jacobschweitzer 8aebc0c
Add common container class for sources
jacobschweitzer 9ae7e58
Add JS for admin tables expand and collapse
jacobschweitzer ffb3fdc
Add margin after source container
jacobschweitzer ab73ddf
Merge branch 'develop' into 1362-improved_url_listing_view
jacobschweitzer 4f14433
Remove todo
jacobschweitzer bda4140
Change wording: Removed to Invalid
jacobschweitzer c5ad3e6
Wrap in <code> and add brackets for attributes
jacobschweitzer 4ea9638
Fix translation function reference
jacobschweitzer 392ee24
Add double arrow to Sources column
jacobschweitzer 163908e
Add hide all sources functionality and styles
jacobschweitzer 0ad0cf6
Change new status icons based on AMP options
jacobschweitzer d4b586f
Combine duplicated post_row_actions filters; ensure only relevant lin…
westonruter 1ab6bd3
Re-use AMP_Validation_Manager::is_sanitization_forcibly_accepted()
westonruter 7eafdc6
Add view errors by type title link button
jacobschweitzer 5b9b243
Change title of page and highlight word URL
jacobschweitzer 52babee
Bolder font weight for page title action link keyword
jacobschweitzer 8f88d0c
ESLint fixes
jacobschweitzer 1d95cae
Fix PHP 5.3 compatibility issues
jacobschweitzer a4bdee9
Update tests to reflect changes
jacobschweitzer fe7e4de
Update to tests and add an error check
jacobschweitzer 6936f05
Address unit test errors
kienstra 4cbee48
Merge branch 'develop' into 1362-improved_url_listing_view
kienstra 2ce2e5e
Addres PHPCS warning by aligning =
kienstra 74feb85
Get full plugin names and fix theme source output
jacobschweitzer 5baae3f
Code review changes and text changes
jacobschweitzer 6911305
Unify JS for text expansion and update styles
jacobschweitzer 56915da
Implement workaround to fix JS not loading issue
jacobschweitzer 6488c05
Update unit tests based on latest changes
jacobschweitzer 88e7b7e
Merge branch 'develop' into 1362-improved_url_listing_view
jacobschweitzer 145a9ef
Fix test listing amp plugin
jacobschweitzer 6624e68
Fix unit tests
jacobschweitzer 716a869
Apply remaining changes from code review
westonruter File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| .column-error_status .tooltip { | ||
| display: none; | ||
| } | ||
| .column-error_status .dashicons-editor-help { | ||
| color: #767676; | ||
| } | ||
| .column-sources_with_invalid_output .dashicons { | ||
| margin-right: 5px; | ||
| } | ||
| .column-sources_with_invalid_output .dashicons-admin-plugins { | ||
| color: #64a2e9; | ||
| } | ||
| .column-sources_with_invalid_output .dashicons-admin-appearance { | ||
| color: #ebb04f; | ||
| } | ||
| .column-sources_with_invalid_output .dashicons-wordpress-alt { | ||
| color: #92b371; | ||
| } | ||
| .sources-plugins, .sources-core { | ||
| margin-left: 25px; | ||
| margin-bottom: 15px; | ||
| } | ||
| .amp-logo-icon { | ||
| background-image: url( '../images/amp-logo-icon.svg' ); | ||
| background-color: transparent; | ||
| background-size: 20px 20px; | ||
| height: 20px; | ||
| width: 20px; | ||
| display: inline-block; | ||
| } | ||
| .column-error_status .error-status { | ||
| line-height: 20px; | ||
| display: inline-block; | ||
| position: relative; | ||
| vertical-align: top; | ||
| margin-left: 10px; | ||
| } | ||
| td.column-found_elements_and_attributes { | ||
| color: #c06e60; | ||
| } | ||
| .column-error_status .dashicons-flag.new { | ||
| color: #d98501; | ||
| } | ||
| .column-error_status .dashicons-yes.new { | ||
| color: #ff0000; | ||
| } | ||
| .column-error_status .dashicons-warning.rejected { | ||
| color: #68c6ff; | ||
| } | ||
| .column-sources_with_invalid_output .sources-plugins.collapsed, .column-sources_with_invalid_output .sources-core.collapsed { | ||
| display: none; | ||
| } | ||
| .column-sources_with_invalid_output .source { | ||
| margin-bottom: 10px; | ||
| } | ||
| .column-sources_with_invalid_output .source { | ||
| margin-bottom: 10px; | ||
| display: block; | ||
| } | ||
| .dashicons.toggle-sources { | ||
| cursor: pointer; | ||
| float: right; | ||
| margin-right: 20%; | ||
| } | ||
| .column-sources_with_invalid_output .double-arrow { | ||
| height: 20px; | ||
| width: 12px; | ||
| display: inline-block; | ||
| position: relative; | ||
| float: right; | ||
| margin-right: 23%; | ||
| cursor: pointer; | ||
| } | ||
| .column-sources_with_invalid_output .double-arrow .dashicons.top-arrow { | ||
| font-size: 20px; | ||
| line-height: 1; | ||
| position: absolute; | ||
| bottom: 0; | ||
| } | ||
| .column-sources_with_invalid_output .double-arrow .dashicons.bottom-arrow { | ||
| font-size: 20px; | ||
| line-height: 1; | ||
| position: absolute; | ||
| bottom: -6px; | ||
| } | ||
| .wrap .wp-heading-inline + .page-title-action { | ||
| margin-left: 1rem; | ||
| } | ||
| .page-title-action strong { | ||
| font-weight: 900; | ||
| } |
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| /* global jQuery, ampAdminTables */ | ||
| ( function( $ ) { | ||
| 'use strict'; | ||
|
|
||
| var adminTables = { | ||
|
|
||
| load: function() { | ||
| $( document ).ready( $.proxy( function() { | ||
| this.sourcesHideShow(); | ||
| this.allSourcesHideShow(); | ||
| this.addViewErrorsByTypeLinkButton(); | ||
| }, this ) ); | ||
| }, | ||
|
|
||
| sourcesHideShow: function() { | ||
| $( 'span.dashicons.toggle-sources' ).on( 'click', function() { | ||
| $( this ).next( '.sources-container' ).toggleClass( 'collapsed' ); | ||
| $( this ).toggleClass( 'dashicons-arrow-down dashicons-arrow-up' ); | ||
| } ); | ||
| }, | ||
|
|
||
| allSourcesHideShow: function() { | ||
| $( '.double-arrow' ).on( 'click', function() { | ||
| $( '.double-arrow' ).find( '.dashicons' ).toggleClass( 'dashicons-arrow-down dashicons-arrow-up' ); | ||
| $( 'span.dashicons.toggle-sources' ).toggleClass( 'dashicons-arrow-down dashicons-arrow-up' ); | ||
| $( '.sources-container' ).toggleClass( 'collapsed' ); | ||
| } ); | ||
| }, | ||
|
|
||
| addViewErrorsByTypeLinkButton: function() { | ||
| var link = $( '<a></a>' ); | ||
| link.prop( 'text', ampAdminTables.errorIndexAnchor ); | ||
| link.prop( 'href', ampAdminTables.errorIndexLink ); | ||
| link.prop( 'class', 'page-title-action' ); | ||
| $( '.wp-heading-inline' ).after( link ); | ||
| } | ||
| }; | ||
|
|
||
| adminTables.load(); | ||
| }( jQuery ) ); | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some of the code here might be duplicated with https://github.com/Automattic/amp-wp/blob/develop/assets/src/amp-validation-error-detail-toggle.js
Can we unify them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@westonruter I started working on this but realized a dependency of the script you mentioned is
wp-dom-readywhich is a part of Gutenberg. So this script doesn't load for me since on my test site Gutenberg is not activated. Are we assuming that everyone will have Gutenberg as well or why is this a dependency? Or did I miss something in the build process?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @westonruter and @jacobschweitzer,
Do you think we should add
dom-readyto package.json, like howi18nis included there:Still,
npm run devseemed to build amp-validation-error-detail-toggle.js fine in my local, and that script ran without error on the 'AMP Validation Error Index' page.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kienstra I've added that line to
package.jsonand rannpm run build/npm run devbut the script still won't load on the Error Index page. Am I missing something here? I did have to install webpack and wp-cli but I have those now so I'm not sure if there is another dependency I'm missing since there aren't any errors in the build process now.