Conversation
Summary of ChangesHello @remcotolsma, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request modernizes the integration of the Adyen Web SDK by transitioning from a direct script loading mechanism to an npm-managed dependency. This change streamlines dependency management, improves the build process, and ensures that the Adyen components are loaded and utilized as modules within the application's JavaScript, enhancing maintainability and consistency. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request updates the Adyen Web integration to use an npm package instead of a CDN script, which is a good modernization. The changes to use the npm package are mostly correct, but I've identified a couple of follow-up changes required in the PHP code. There's a call to print a script that is no longer registered, which will cause an error. Additionally, there's now a version mismatch between the JavaScript library and the stylesheet being loaded, which could lead to visual bugs. My review includes details on how to fix these issues.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the Adyen Web integration to use the @adyen/adyen-web NPM package instead of loading the script from a CDN. This is a great improvement for managing dependencies. The changes involve updating the JavaScript to import from the package, adding the dependency to package.json, and updating the PHP code to remove the old script registration. The code is well-structured, but I have one suggestion to make it more robust when handling script dependencies.
src/Gateway.php
Outdated
| 'pronamic-pay-adyen-checkout-drop-in', | ||
| \plugins_url( '../js/dist/checkout-drop-in.js', __FILE__ ), | ||
| [ 'pronamic-pay-adyen-checkout' ], | ||
| [], |
There was a problem hiding this comment.
While the dependencies array in checkout-drop-in.asset.php is currently empty, it's a best practice to use the dependencies array from the asset file here instead of hardcoding an empty array. This makes the code more robust and future-proof. If wp-scripts adds any dependencies in the future (e.g., from other @wordpress packages), they will be automatically handled without needing to change this line.
$asset['dependencies'],
No description provided.