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.

Sign up for GitHub

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

Jump to bottom

Plugins: Add Action Links to Plugin Detail View #11718

Merged
merged 2 commits into from
Mar 10, 2017
Merged

Plugins: Add Action Links to Plugin Detail View #11718

merged 2 commits into from
Mar 10, 2017

Conversation

roundhill
Copy link
Contributor

@roundhill roundhill commented Mar 2, 2017 •
edited
Loading

Adds plugin action links buttons to the plugin detail view. The plugin developer adds these using the action link filter.

To test

  • Ensure your Jetpack site is running 4.7.
  • Install some plugins via calypso that use action links (and some that don't). Jetpack and WooCommerce are good examples.
  • After installing the plugin, you should see the action links appear as buttons underneath the plugin icon.
  • Test the buttons to make sure they link properly to the content.
  • Remove the plugin. The buttons should disappear.

Screenshot:
screen shot 2017-02-28 at 3 27 23 pm

Fixes #82

@roundhill roundhill added [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Mar 2, 2017
@matticbot
Copy link
Contributor

Test live: https://calypso.live/?branch=add/82-plugin-action-links

@matticbot matticbot added the [Size] S Small sized issue label Mar 2, 2017
@davewhitley davewhitley self-requested a review March 6, 2017 18:11
@davewhitley davewhitley removed the [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR label Mar 6, 2017
davewhitley
davewhitley approved these changes Mar 6, 2017
View reviewed changes
Copy link
Contributor

@davewhitley davewhitley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Design looks good.

@dmsnell
Copy link
Member

dmsnell commented Mar 9, 2017 •
edited
Loading

code looks good to me @roundhill but I haven't tested it yet

a cursory search showed no conflicts with letting through the additional key

@apeatling apeatling self-requested a review March 9, 2017 20:05
apeatling
apeatling approved these changes Mar 9, 2017
View reviewed changes
Copy link
Member

@apeatling apeatling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 on the design

@roundhill
Copy link
Contributor Author

Note: You can now test this with production Jetpack (4.7). I updated the testing instructions.

vindl
vindl reviewed Mar 10, 2017
View reviewed changes
client/my-sites/plugins/plugin-meta/index.jsx Outdated
{ ! isEmpty( actionLinks ) &&
<div className="plugin-meta__action-links">
{ Object.keys( actionLinks ).map( linkTitle => (
<Button compact icon href={ actionLinks[ linkTitle ] } target="_blank">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should add rel="noopener noreferrer" here. See #7680.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I wasn't aware of that. Added in 5f046dd

vindl
vindl approved these changes Mar 10, 2017
View reviewed changes
Copy link
Member

@vindl vindl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested it and it looks good to me. I've left a small comment about target="_blank", and I think this is good to :shipit: after that is resolved.

@matticbot matticbot added [Size] M Medium sized issue and removed [Size] S Small sized issue labels Mar 10, 2017
@vindl vindl added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Mar 10, 2017
@roundhill roundhill merged commit e0cce68 into master Mar 10, 2017
@roundhill roundhill deleted the add/82-plugin-action-links branch March 10, 2017 19:21
@roundhill roundhill mentioned this pull request Mar 14, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@apeatling apeatling apeatling approved these changes

@vindl vindl vindl approved these changes

@davewhitley davewhitley davewhitley approved these changes

Assignees
No one assigned
Labels
[Size] M Medium sized issue
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@roundhill @matticbot @dmsnell @davewhitley @vindl @apeatling