-
Notifications
You must be signed in to change notification settings - Fork 1
Enhance eCash Address Configuration Flow #37
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
Conversation
| /* ------------------------------ | ||
| Paywall Settings page styles | ||
| ------------------------------ */ | ||
| #ecashAddressValidationResult { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make the address concept generic since we do have support for other blockchains.
#addressValidationResult or #paymentAddressValidationResult would be good.
| public function __construct() { | ||
| add_action( 'admin_menu', array( $this, 'add_admin_menus' ) ); | ||
| add_action( 'admin_enqueue_scripts', array( $this, 'enqueue_admin_scripts' ) ); | ||
| add_action( 'admin_notices', array( $this, 'admin_notice_missing_ecash_address' ) ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
| $this->load_admin_template( 'paywall-settings', $args ); | ||
| } | ||
|
|
||
| public function admin_notice_missing_ecash_address() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
admin_notice_missing_address or admin_notice_missing_payment_address
| if (!current_user_can('manage_options')) { | ||
| return; | ||
| } | ||
| $address = get_option('paybutton_paywall_ecash_address', ''); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
| $address = get_option('paybutton_paywall_ecash_address', ''); | ||
| if (empty($address)) { | ||
| echo '<div class="notice notice-error">'; | ||
| echo '<p><strong>PayButton:</strong> Please set your eCash Address in <a href="' . esc_url(admin_url('admin.php?page=paybutton-paywall')) . '">Paywall Settings</a>. If you don\'t have an eCash Address yet, create a wallet using <a href="https://cashtab.com">Cashtab</a> or <a href="https://www.bitcoinabc.org/electrum/">Electrum ABC</a>.</p>'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
We can link to Cashtab and Electrum ABC for XEC, and Electron Cash for BCH. Wording would need some updating to make it clear that we support both.
I think in the future if we do expand to other blockchains, we'll have to improve the text layout a bit to make it more manageable as we don't want the banner at the top taking up so much room; although maybe it's fine since we want them to address the "error" anyway.
| <tr> | ||
| <th scope="row"><label for="ecash_address">eCash Address</label></th> | ||
| <td><input type="text" name="ecash_address" id="ecash_address" class="regular-text" value="<?php echo esc_attr( $ecash_address ); ?>"></td> | ||
| <th scope="row"><label for="ecash_address">eCash Address (required)</label></th> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
| <td><input type="text" name="ecash_address" id="ecash_address" class="regular-text" value="<?php echo esc_attr( $ecash_address ); ?>"></td> | ||
| <th scope="row"><label for="ecash_address">eCash Address (required)</label></th> | ||
| <td> | ||
| <input type="text" name="ecash_address" id="ecash_address" class="regular-text" value="<?php echo esc_attr( $ecash_address ); ?>" required> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
| <td> | ||
| <input type="text" name="ecash_address" id="ecash_address" class="regular-text" value="<?php echo esc_attr( $ecash_address ); ?>" required> | ||
| <!-- This span will be populated by our bundled address validator JS --> | ||
| <span id="ecashAddressValidationResult"></span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
| if ( ! defined( 'ABSPATH' ) ) exit; // Exit if accessed directly | ||
|
|
||
| // Check if the admin has set an eCash address | ||
| $admin_ecash_address = get_option('paybutton_paywall_ecash_address', ''); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
|
|
||
| // Check if the admin has set an eCash address | ||
| $admin_ecash_address = get_option('paybutton_paywall_ecash_address', ''); | ||
| if ( empty( $admin_ecash_address ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
|
You can merge this PR as-is. In a subsequent PR, I will update the payment address naming across multiple files to ensure consistency and multi-chain (XEC and BCH) support. |
Klakurka
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok I'll approve it this time but as a general rule, you always want to be iterating towards something and part of that means not removing/changing things that you know you will need to change very soon after.
This PR fixes #36 and introduces several changes to improve the configuration experience and admin functionality of the PayButton plugin:
Test plan: