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

Added customer introduction to the paytrail payments page #130

Closed
wants to merge 1 commit into from

Conversation

Taiteilija
Copy link
Collaborator

Also replaced some compiling packets since the ones in the plugin were no longer supported

Also replaced some compiling packets since the ones in the plugin were no longer supported
};

const closeOverlay = () => {
console.log('Closing overlay');
Copy link
Collaborator

Choose a reason for hiding this comment

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

should remove this

const credentialButton = document.getElementById('credentials');
const testModeButton = document.getElementById('test-mode-button');
const testModeCheckbox = document.getElementById('woocommerce_paytrail_enable_test_mode');
const mainForm = document.getElementById('mainform');
Copy link
Collaborator

Choose a reason for hiding this comment

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

mainForm does not appear to be used

* Enqueue jQuery from WordPress core
*/
public function enqueue_jquery() {
wp_enqueue_script('jquery');
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this can be improved by the checking which admin page is in the $hook_suffix and ensuring we only add the scripts to the necessary pages. https://developer.wordpress.org/reference/hooks/admin_enqueue_scripts/

/**
* Enqueue jQuery UI from WordPress core
*/
public function enqueue_jquery_ui() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as comment on enqueue_jquery()

@@ -1932,40 +2141,66 @@ public function handle_custom_searches( $query, $query_vars) {
return $query;
}

public function enqueue_admin_scripts() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

As per comments on plugin.php, there is the $hook_suffix parameter passed to this that can be used to only action on the required pages.

},
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1",
"watch": "webpack --watch",
"dev": "webpack",
"build": "webpack -p"
"build": "webpack"
Copy link
Collaborator

Choose a reason for hiding this comment

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

See my comment on webpack.config.js, I'm not familiar with it so just noting I've not reviewed changes related to this

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can't review due to the large diff size. So just nothing that. I'm not familiar with what this file is used for either.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Another I can't review due to the size change diff and also not familiar with the file

Copy link
Collaborator

Choose a reason for hiding this comment

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

Another I can't review due to the size change diff and also not familiar with the file

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is just auto generated so not reviewed.

@Taiteilija Taiteilija closed this Apr 16, 2024
@Taiteilija Taiteilija deleted the customer-introduction branch April 16, 2024 12:26
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

3 participants