Skip to content
This repository has been archived by the owner on Oct 10, 2018. It is now read-only.

Add styles for Recent Comments widget #28

Closed
westonruter opened this issue Jan 16, 2018 · 5 comments
Closed

Add styles for Recent Comments widget #28

westonruter opened this issue Jan 16, 2018 · 5 comments
Labels
Milestone

Comments

@westonruter
Copy link
Contributor

The Recent Comments widget outputs the following style element in the head via \WP_Widget_Recent_Comments::recent_comments_style():

<style type="text/css">.recentcomments a{display:inline !important;padding:0 !important;margin:0 !important;}</style>

The show_recent_comments_widget_style filter allows this to be turned off, which it must be for AMP validity. Since it is off, the theme itself will need to incorporate this CSS rule, except for !important since this is also invalid in AMP.

Ref. ampproject/amp-wp#864

@kopepasah
Copy link
Contributor

Ready for review in #54.

@MackenzieHartung MackenzieHartung added this to the v0.1 milestone Feb 5, 2018
@kienstra
Copy link
Contributor

Steps To Test

  1. Go to a test page with a 'Recent Comments' widget
  2. Verify that the comments display
  3. Verify that clicking them leads to a post

@csossi
Copy link

csossi commented Feb 20, 2018

@kienstra , Could there be some sort of cue for user to resolve error re: URL format here?
image

@westonruter
Copy link
Contributor Author

Yes, there could. I think if you submit this as-is, however, AMP there will be an error displayed at that time, will it not? In this case here, the field is marked as url and so the browser is reporting it as invalid. However, WordPress on the backend will correctly prepend it with http:// so no error will ensue. We could add more error reporting via https://www.ampproject.org/docs/reference/components/amp-form#custom-validations however I think this is beyond the scope of just adding commenting support. WordPress itself doesn't provide contextual error reporting if you supply a bad URL.

@csossi
Copy link

csossi commented Feb 21, 2018

understood, @westonruter - noting this as verified in QA - cheers

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

6 participants