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

Add links to documentation throughout the plugin settings #1363

Merged
merged 20 commits into from Dec 28, 2016
Merged

Add links to documentation throughout the plugin settings #1363

merged 20 commits into from Dec 28, 2016

Conversation

mathetos
Copy link
Member

Description

Fixes #890

Adding a new custom field type called "docs-link" to allow for easy and consistent implementation of links to documentation in the form edit screen and the settings panels.

How Has This Been Tested?

locally on PHP 5.6+, WordPress 4.7, on Give branch 1.8 with no Addons activated.

Screenshots (jpeg or gifs if applicable):

Will come later when styled correctly

Types of changes

  • Adding a new custom field type: "docs-link"
  • Adding new fields to form edit screen and settings panels

Checklist:

  • [x ] My code is tested.
  • My code follows the WordPress code style.
  • My code follows has proper inline documentation.

@mathetos
Copy link
Member Author

This is currently incomplete. Will be adding more commits soon.

@mathetos
Copy link
Member Author

Here's the doc-link at the bottom-right of the "Donation Options" panel:
image

And here it is on hover:
image

@mathetos
Copy link
Member Author

Looks like we still use CMB2 for our settings panels, so I'll have to make the additional custom field type for that.

@@ -645,7 +645,7 @@ function give_docs_link( $field ) {
$field['url'] = isset( $field['url'] ) ? $field['url'] : 'https://givewp.com/documentation';
$field['title'] = isset( $field['title'] ) ? $field['title'] : 'Documentation';
Copy link
Contributor

Choose a reason for hiding this comment

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

Make translatable: __( 'Documentation', 'give' );

Copy link
Member Author

Choose a reason for hiding this comment

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

At first I thought we'd be fine since the string is escaped in the field function, but then it wouldn't be properly internationalized. Updating now. Thanks!

'name' => 'donation_options_docs',
'type' => 'docs_link',
'url' => 'https://givewp.com/documentation/core/give-forms/',
'title' => "Documentation Options",
Copy link
Contributor

Choose a reason for hiding this comment

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

I think Documentation Options should be Donation Options and also should be translatable:
__( 'Donation Options', 'give' )

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch.

@@ -645,7 +645,7 @@ function give_docs_link( $field ) {
$field['url'] = isset( $field['url'] ) ? $field['url'] : 'https://givewp.com/documentation';
$field['title'] = isset( $field['title'] ) ? $field['title'] : 'Documentation';

echo '<p class="docs-link"><a href="' . $field['url'] . '" target="_blank">'. esc_html__('Need Help? See docs on ') . $field['title'] . '<span class="dashicons dashicons-editor-help"></span></a></p>';
echo '<p class="docs-link"><a href="' . $field['url'] . '" target="_blank">'. esc_html__('Need Help? See docs on "') . $field['title'] . '" <span class="dashicons dashicons-editor-help"></span></a></p>';
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Escape $field['url'] on output.
  2. sprintf() is recommended when a translatable string includes a variable. This allows the position of the variable within the string to be moved if the translation requires it.

Recommended change:

echo '<p class="docs-link"><a href="'
. esc_url( $field['url'] )
. '" target="_blank">'
. sprintf( esc_html( 'Need Help? See docs on "%s"', 'give' ), $field['title'] )
. ' <span class="dashicons dashicons-editor-help"></span></a></p>';

array(
'name' => 'donation_options_docs',
'type' => 'docs_link',
'url' => 'https://givewp.com/documentation/core/give-forms/',
'title' => "Documentation Options",
'title' => 'Documentation Options',
Copy link
Contributor

Choose a reason for hiding this comment

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

All 'title' fields need to be translatable... Donation Options, Form Display, Donation Goal, etc.

@mathetos
Copy link
Member Author

@DevinWalker This PR is ready for your review. All help links have been added both on the Form Edit screen as well as the Settings panels.

NOTE: We have to re-arrange some docs and write new ones BEFORE we ship 1.8. It's not difficult and it's a good way for us to focus our docs time and improve the docs navigation. Thanks!

@mathetos
Copy link
Member Author

@kevinwhoffman mentioned that I'm escaping twice. WIll update that on Tuesday after the long holiday weekend. Then it's good to go.

@kevinwhoffman
Copy link
Contributor

For reference: In all of the instances where title is defined, we should provide translation but not escape the value until it is output. So 'title' => esc_html__( 'Access Control', 'give' ) should be 'title' => __( 'Access Control', 'give' ). Escaping is already covered just prior to output.


function give_docs_link( $field ) {
$field['url'] = isset( $field['url'] ) ? $field['url'] : 'https://givewp.com/documentation';
$field['title'] = isset( $field['title'] ) ? $field['title'] : 'Documentation';
Copy link
Collaborator

Choose a reason for hiding this comment

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

code alignment is not correct.

'id' => 'advanced_settings_docs_link',
'url' => esc_url( 'http://docs.givewp.com/advancedsettings' ),
'title' => __( 'Advanced Settings', 'give' ),
'type' => 'give_docs_link',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Code alignment is not correct.

'id' => 'display_settings_docs_link',
'url' => esc_url( 'http://docs.givewp.com/displayoptions' ),
'title' => __( 'Display Options Settings', 'give' ),
'type' => 'give_docs_link',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Code alignment is not correct.

'name' => esc_html__( 'Post Types Docs Link', 'give' ),
'id' => 'post_types_settings_docs_link',
'url' => esc_url( 'http://docs.givewp.com/posttypessettings' ),
'title' => __( 'Post Types Settings', 'give' ),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Code alignment is not correct.

'name' => esc_html__( 'Taxonomies Docs Link', 'give' ),
'id' => 'taxonomies_settings_docs_link',
'url' => esc_url( 'http://docs.givewp.com/taxonomiessettings' ),
'title' => __( 'Taxonomies Settings', 'give' ),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Code alignment is not correct.

'name' => esc_html__( 'Terms and Conditions Docs Link', 'give' ),
'id' => 'terms_settings_docs_link',
'url' => esc_url( 'http://docs.givewp.com/termssettings' ),
'title' => __( 'Terms and Conditions Settings', 'give' ),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Code alignment is not correct.

'name' => esc_html__( 'Email Settings Docs Link', 'give' ),
'id' => 'email_settings_docs_link',
'url' => esc_url( 'http://docs.givewp.com/emailsettings' ),
'title' => __( 'Email Settings', 'give' ),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Code alignment is not correct.

'name' => esc_html__( 'Donation Receipt Settings Docs Link', 'give' ),
'id' => 'donation_receipt_settings_docs_link',
'url' => esc_url( 'http://docs.givewp.com/donationreceipt' ),
'title' => __( 'Donation Receipt Settings', 'give' ),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Code alignment is not correct.

'name' => esc_html__( 'Donation Notification Settings Docs Link', 'give' ),
'id' => 'donation_notification_settings_docs_link',
'url' => esc_url( 'http://docs.givewp.com/donationnotification' ),
'title' => __( 'Donation Notification Settings', 'give' ),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Code alignment is not correct.

'name' => esc_html__( 'PayPal Standard Gateway Settings Docs Link', 'give' ),
'id' => 'paypal_standard_gateway_settings_docs_link',
'url' => esc_url( 'http://docs.givewp.com/paypalstandard' ),
'title' => __( 'PayPal Standard Gateway Settings', 'give' ),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Code alignment is not correct.

'name' => esc_html__( 'Offline Donations Settings Docs Link', 'give' ),
'id' => 'offline_gateway_settings_docs_link',
'url' => esc_url( 'http://docs.givewp.com/offlinegateway' ),
'title' => __( 'Offline Gateway Settings', 'give' ),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Code alignment is not correct.

'name' => esc_html__( 'Gateways Docs Link', 'give' ),
'id' => 'gateway_settings_docs_link',
'url' => esc_url( 'http://docs.givewp.com/gatewayssettings' ),
'title' => __( 'Gateway Settings', 'give' ),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Code alignment is not correct.

'name' => esc_html__( 'Access Control Docs Link', 'give' ),
'id' => 'access_control_docs_link',
'url' => esc_url( 'http://docs.givewp.com/accesscontrol' ),
'title' => __( 'Access Control', 'give' ),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Code alignment is not correct.

'name' => esc_html__( 'Currency Options Docs Link', 'give' ),
'id' => 'currency_settings_docs_link',
'url' => esc_url( 'https://docs.givewp.com/currencysettings' ),
'title' => __( 'Currency Settings', 'give' ),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Code alignment is not correct.

'name' => esc_html__( 'General Options Docs Link', 'give' ),
'id' => 'general_options_docs_link',
'url' => esc_url( 'https://docs.givewp.com/generaloptions' ),
'title' => __( 'General Options', 'give' ),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Code alignment is not correct.

),
array(
'name' => 'offline_docs',
'type' => 'docs_link',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Extra spaces in array. It is not an issue but we can reduce them

@mathetos
Copy link
Member Author

@ravinderk All alignment issues have been addressed. Thanks!

@DevinWalker DevinWalker merged commit 539273d into impress-org:release/1.8 Dec 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants