Register scripts during *_enqueue_scripts hook instead #368
Conversation
| */ | ||
| public function login_enqueue_assets() { |
There was a problem hiding this comment.
This wasn't used anywhere.
| ); | ||
|
|
||
| wp_register_script( | ||
| 'fido-u2f-login', |
There was a problem hiding this comment.
This is used during our custom "login" page https://github.com/wordpress/two-factor/blob/a97c32b6639a3557fd149725b973615246549ffe/class-two-factor-core.php#L581-L726
| wp_enqueue_script( 'fido-u2f-login' ); | ||
| public static function enqueue_scripts() { | ||
| wp_register_script( | ||
| 'fido-u2f-api', |
There was a problem hiding this comment.
This is a dependency for fido-u2f-admin and fido-u2f-login.
| true | ||
| ); | ||
| // Ensure the script dependencies have been registered before they're enqueued at a later priority. | ||
| add_action( 'admin_enqueue_scripts', array( __CLASS__, 'enqueue_scripts' ), 5 ); |
There was a problem hiding this comment.
Use action priority 5 to ensure the scripts have been registered before they're enqueued later at priority 10.
|
This looks good, however, is there a particular reason why we do script registration and enqueueing separately? I may be missing some context, but it seems to me that the complexity might be reduced a bit if combined into a single |
|
@rinatkhaziev That's a good question. I think the reason for that is that both admin and login screens rely on the same JS dependencies so we're able to reference those dependencies without having to enqueue them two multiple times and potentially mess up the file paths, versions, etc. |
|
Ah I see, thanks! |
Fixes #361.