Add external link icon for none wporg themes and plugins #6704
Conversation
|
Plugin builds for 24c4948 are ready 🛎️!
|
Codecov Report
@@ Coverage Diff @@
## develop #6704 +/- ##
=============================================
- Coverage 76.67% 76.62% -0.06%
+ Complexity 6543 6520 -23
=============================================
Files 263 263
Lines 20955 20870 -85
=============================================
- Hits 16068 15991 -77
+ Misses 4887 4879 -8
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
The PR looks good to me 👍
I'm just wondering if we could use a similar markup to the markup used in Core for external links. E.g. on the WP admin Dashboard, in the Events and News panel, there are a few external links:
Here's a markup of one of them:
<a href="https://make.wordpress.org/community/meetups-landing-page" target="_blank">Meetups <span class="screen-reader-text">(opens in a new tab)</span><span aria-hidden="true" class="dashicons dashicons-external"></span></a>It's very similar to what we have but it additionally contains the screen reader text and the aria-hidden property.
dhaval-parekh
force-pushed
the
enhancement/2313-highlight-amp-compatible-theme-plugin
branch
from
cc3f180 to
b951ba0
Compare
|
@delawski It's a good suggestion. I have added screen reader text along with the |
|
@dhaval-parekh noticed a small issue, Themes that are present on wordpress.org have no
|
|
@jamesozzie we may also need to correct plugin URLs for some plugins eg: site kit by google, Setka Editor are pointing to external sites (thier official site) they should be pointing to wp.org plugin link.
|
|
@milindmore22 I think you're looking at an old version of the plugin. I corrected this. In the latest version, the AMP Compatible banner is also removed in favor of a badge but not on the AMP-Compatible tab.
|
| .theme-browser .theme { | ||
| float: none; | ||
| display: inline-block; | ||
| vertical-align: top; | ||
| } | ||
|
|
There was a problem hiding this comment.
This style rule didn't seem to be doing anything anymore. I think it was needed for the AMP-compatible banner which we replaced with a badge. @dhaval-parekh is that right?
There was a problem hiding this comment.
Yes, This style is no longer needed. It was added for the AMP-compatible banner.
| .theme-browser .theme .theme-screenshot img { | ||
| height: 100%; | ||
| object-fit: contain; | ||
| } |
There was a problem hiding this comment.
This is needed because the theme screenshots from amp-wp.org are not the same aspect ratio as those from WordPress.org:
| Before | After |
|---|---|
 |
 |
I don't see that issue:
|
westonruter
deleted the
enhancement/2313-highlight-amp-compatible-theme-plugin
branch
|
Oops!! yes, you are correct I didn't pull the latest changes! all looks good now 👍🏼 |
|
|
||
| if ( ! empty( $plugin['homepage'] ) ) { | ||
| $actions[] = sprintf( | ||
| '<a href="%s" target="_blank" rel="noopener noreferrer" aria-label="%s">%s</a>', | ||
| '<a href="%s" target="_blank" rel="noopener noreferrer" aria-label="%s">%s<span class="screen-reader-text">(opens in a new tab)</span>%s</a>', |
There was a problem hiding this comment.
I just realized this is missing translation. See #6723.
|
QA Passed The external icons are correctly shown when installing a plugin or a theme.
This has been QA'ed as part of #6725 (comment) |
westonruter
added
the
Changelogged
Summary
Related #2313
This PR adds an external link icon in the "Visit Site" button for none WordPress org plugin/theme in the "AMP Compatible" tab to indicate that the link may go to another site and not to the WordPress.org site.
Add new theme page
Add new plugin page
Checklist