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

Removing the handheld footer menu causes js error: Uncaught ReferenceError: storefrontScreenReaderText is not defined #2082

Open
mikezielonka opened this issue Dec 22, 2022 · 4 comments
Labels
good first issue The issue is a good candidate for the first community contribution/for a newcomer to the team.

Comments

@mikezielonka
Copy link

Hi All! I removed the handheld footer menu using the documented process.

Screenshot by Dropbox Capture

Removing the footer menu then results in a JS error in the browser

navigation.min.js?ver=4.2.0:formatted:24 Uncaught ReferenceError: storefrontScreenReaderText is not defined at navigation.min.js?ver=4.2.0:formatted:24:59 at NodeList.forEach (<anonymous>) at HTMLDocument.<anonymous> (navigation.min.js?ver=4.2.0:formatted:18:48)

// Remove the handheld footer menu
add_action( 'init', 'ddoo_remove_storefront_handheld_footer_bar' );

function ddoo_remove_storefront_handheld_footer_bar() {
  remove_action( 'storefront_footer', 'storefront_handheld_footer_bar', 999 );
}
@imanish003
Copy link

Hi @mikezielonka, Thanks for creating the issue – much appreciated! 🙏

I followed the documented process, and it's working fine for me.

Can you please share the steps to reproduce the issue including how/where you added following lines?

// Remove the handheld footer menu
add_action( 'init', 'ddoo_remove_storefront_handheld_footer_bar' );

function ddoo_remove_storefront_handheld_footer_bar() {
 remove_action( 'storefront_footer', 'storefront_handheld_footer_bar', 999 );
}

@stefpb
Copy link

stefpb commented Jan 5, 2023

I have the same behavior. The footer.min.js is still loading. Look at the source code (right click in browser -> view page source):

<script src='/wp-content/themes/storefront/assets/js/footer.min.js?ver=4.2.0' id='storefront-handheld-footer-bar-js'></script>

You should also remove the script by

	wp_dequeue_script('storefront-handheld-footer-bar');

@sunyatasattva sunyatasattva added the needs feedback The issue/PR needs a response from any of the parties involved in the issue. label Feb 3, 2023
@sunyatasattva
Copy link
Contributor

Interesting! Thanks @stefpb for the insight. @danielwrobert, I wonder if we should:

  1. Add script enqueueing directly within the storefront_handheld_footer_bar function, so that when this is removed via an action, the script should get removed as well.
  2. Add the instructions to dequeue the script within the docs.

Thoughts?

@Aljullu Aljullu added the good first issue The issue is a good candidate for the first community contribution/for a newcomer to the team. label Feb 13, 2023
@Aljullu
Copy link
Contributor

Aljullu commented Feb 13, 2023

Agree with both solutions. I specially like 1 because it would keep the snippet simple as-is. In the meanwhile, the solution proposed by @stefpb above, works.

I added the good first issue label to this issue, in case somebody wants to contribute a fix.

Currently the script is enqueued here:

wp_enqueue_script( 'storefront-handheld-footer-bar', get_template_directory_uri() . '/assets/js/footer' . $suffix . '.js', array(), $storefront_version, true );

and with the proposed solution it would be enqueued inside this function:

function storefront_handheld_footer_bar() {

We would need to test that it doesn't cause any regressions, though.

@Aljullu Aljullu removed the needs feedback The issue/PR needs a response from any of the parties involved in the issue. label Feb 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue The issue is a good candidate for the first community contribution/for a newcomer to the team.
Projects
None yet
Development

No branches or pull requests

5 participants