-
Notifications
You must be signed in to change notification settings - Fork 27
Adding custom filter for one-tap login #282
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
base: develop
Are you sure you want to change the base?
Conversation
rtBot
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.
Code analysis identified issues
action-phpcs-code-review has identified potential problems in this pull request during automated scanning. We recommend reviewing the issues noted and that they are resolved.
phpcs scanning turned up:
🚫 4 errors
Powered by rtCamp's GitHub Actions Library
mi5t4n
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.
I've left a few comments. Please feel free to reach out if you have any questions.
| * @return void | ||
| */ | ||
| public function maybe_add_one_tap_to_frontend(): void { | ||
| $show_on_this_page = apply_filters( |
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.
This filter doesn’t appear to be working. Even when it returns false, the file build/js/onetap.js is still being loaded on every page.
Steps to reproduce
- Goto the
Login With GoogleSettings under the WPSettingsmenu. - Populate the required credentials like
Client IDandClient Secret. - Checked the
Create New UserandEnable One Tap Login - Select the
Enable One Tap Login Site-Wideoption - Add below snippet code in
Hello Dollyplugin or the in the current theme's functions.php file.
add_filter( 'rtcamp.google_one_tap_show', function( $is_allowed ) {
return ! is_home();
});- Navigate to the homepage
- We can still see the onetap pop-up view
Code that is probably causing the above issue
login-with-google/src/Modules/OneTapLogin.php
Lines 173 to 190 in 2a11dd6
| wp_register_script( | |
| 'login-with-google-one-tap-js', | |
| trailingslashit( plugin()->url ) . 'assets/build/js/' . $filename, | |
| [ | |
| 'wp-i18n', | |
| ], | |
| filemtime( trailingslashit( plugin()->path ) . 'assets/build/js/onetap.js' ), | |
| true | |
| ); | |
| wp_add_inline_script( | |
| 'login-with-google-one-tap-js', | |
| 'var TempAccessOneTap=' . json_encode( $data ), //phpcs:disable WordPress.WP.AlternativeFunctions.json_encode_json_encode | |
| 'before' | |
| ); | |
| wp_enqueue_script( 'login-with-google-one-tap-js' ); |
| * | ||
| * @return void | ||
| */ | ||
| public function maybe_add_one_tap_to_frontend(): void { |
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.
Add @since n.e.x.t to newly added function.
| 'rtcamp.google_one_tap_show', | ||
| ( 'sitewide' === $this->settings->one_tap_login_screen ) | ||
| ); | ||
|
|
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.
Another thing, we have to version and document the newly added filter as well.
Description
Fixes
Add Ability to Render One-Tap Login Code/JS on Specific Pages with Filters (#279)