-
Notifications
You must be signed in to change notification settings - Fork 12
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
Feedback: Unexpected links #31
Comments
Another example report, the theme has theme/author URI but the credit link does not match |
I am wondering how to best handle the list of social media links. Whatsapp is not listed in https://github.com/WordPress/theme-review-action/blob/trunk/actions/ui-check/tests/e2e/specs/page/unexpected-links/index.js#L19 Whatsapp: Wordpress.com Are there any other reasons for reducing tellyworth.wordpress.com to just wordpress.com?
https://themes.trac.wordpress.org/ticket/97852 GitHub. You can blame @aristath for this one. |
This report is for a block theme, so we can be assured it works for them 👍 |
This is a child theme, this seems like a false positive: |
No, this is a problem with the parent theme:
|
Are you suggesting we whitelist these for now?
Adding @tellyworth since he's may be more familiar with the reasoning... |
Yes. These links should be allowed because simple social sharing links are allowed. |
I've added the links in 58ca179. We still need a better approach for Theme URI/Author URIs so i'll leave this ticket open. |
Does this also check for "No remote resources are allowed. Include all scripts, images, videos and other resources rather than hot-linking."? I mean if there is an allowed list, then the CDN check is not needed? https://github.com/WordPress/theme-check/blob/master/checks/class-cdn-check.php |
Unexpected links
It is not 100% clear from the report or docs which requirement it refers to.
I believe it is this one?
Themes may have a single footer credit link, which is restricted to the Theme URI or Author URI defined in style.css
Is it possible to include an explanation in the report, that the link is not approved because it is not also listed as the theme or author URI?
I do not understand the code well enough to see where this is confirmed.
Example report:
https://themes.trac.wordpress.org/ticket/97704#comment:10
At the point of the report, theme did not have a theme or author URI but credit links:
I would also suggest that the requirement itself is changed to
Themes can include one single front facing credit link, which is restricted to the Theme URI or Author URI defined in style.css.
I will bring that up with the themes team.
The text was updated successfully, but these errors were encountered: