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

Re-use styling for unmoderated comments to apply to new accepted/rejected validation errors #1458

Merged
merged 7 commits into from Sep 26, 2018

Conversation

@westonruter
Copy link
Member

@westonruter westonruter commented Sep 24, 2018

WordPress has a convention for marking items that need to be actioned on, namely for comments that have not been approved yet. This styling would seem to make sense for new validation errors as well:

image

This would help draw attention to the validation errors that are new which the user needs to action on. Now that the number of rows that have this styling would correspond to the error count appearing with Error Index admin menu item:

image

Aside: The styling above was actually implemented in 58d03c9 but it got lost in dead code that no longer was loaded (amp-validation-error-detail-toggle.js).

TODO

  • Also apply the same styling to new validation errors appearing on the invalid URL screen, making sure that the style is removed as soon as a user changes a select to “Accepted” or “Rejected”.
  • Style invalid URLs in the post list table with unapproved style of contain new errors.
@westonruter westonruter added this to the v1.0 milestone Sep 24, 2018
@johnwatkins0
Copy link
Contributor

@johnwatkins0 johnwatkins0 commented Sep 24, 2018

Hi, @westonruter. @kienstra asked me to handle

Also apply the same styling to new validation errors appearing on the invalid URL screen, making sure that the style is removed as soon as a user changes a select to “Accepted” or “Rejected”.

I pushed an update that applies the unmoderated comment styles to new errors in the single URL table (both the main row and the expanded details row when it's open). I'm removing the [WIP] from this PR now.

@johnwatkins0 johnwatkins0 changed the title [WIP] Re-use styling for unmoderated comments to apply to new accepted/rejected validation errors Re-use styling for unmoderated comments to apply to new accepted/rejected validation errors Sep 24, 2018
@westonruter westonruter changed the title Re-use styling for unmoderated comments to apply to new accepted/rejected validation errors [WIP] Re-use styling for unmoderated comments to apply to new accepted/rejected validation errors Sep 24, 2018
@westonruter
Copy link
Member Author

@westonruter westonruter commented Sep 24, 2018

@johnwatkins0 I added one more todo. In the list of Invalid URLs, we should also show the same styling for Invalid URLs that contain any new validation errors.

const statusText = select.options[ select.selectedIndex ].innerText.trim();

if ( statusText ) {
row.classList.toggle( 'new', 'New Rejected' === statusText || 'New Accepted' === statusText );
Copy link
Member Author

@westonruter westonruter Sep 24, 2018

@johnwatkins0 This will break when a different locale is active. Instead of looking at the option text, you should use the option value which is an integer which will not change.

@jacobschweitzer
Copy link
Contributor

@jacobschweitzer jacobschweitzer commented Sep 25, 2018

@westonruter I've updated this based on the new to-do and your latest comment. Could you take another look at this please? Thanks!

@westonruter westonruter changed the title [WIP] Re-use styling for unmoderated comments to apply to new accepted/rejected validation errors Re-use styling for unmoderated comments to apply to new accepted/rejected validation errors Sep 26, 2018
@westonruter westonruter merged commit 038301f into develop Sep 26, 2018
2 checks passed
@westonruter westonruter deleted the add/unmoderated-row-styling branch Sep 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants