Support new+old Google analytics

[FabioRosado]

Hello, first of all, thank you for the awesome theme!

This PR is an attempt at a fix #385 so we can support the new google ids if they are in use. Please let me know if this looks good or would you rather change the if logic?

I need to look at the tests that we currently have in the project. Once I understand what is happening, I will mark this PR as ready for review 😄 👍

[choldgraf]

At a first glance this looks great to me :-)

[jorisvandenbossche]

Yes, looks good to me as well

[FabioRosado]

Thank you for your comments! I've added two tests, but they are rather dependent on the place where the script will be placed in the page. Hopefully this will be alright, if not I can find a different way to do it or if you have a better idea 😄

[choldgraf]

I think this looks great, wanna mark your PR as "ready for review" when you think it's good to go?

@jorisvandenbossche I'm curious what you think about putting template logic like this into Python functions that we then call from the HTML template. In this way, we can test more coverage of functionality like this, because the codecov would catch the Python lines (while it won't catch the template lines)

[FabioRosado]

Oops thank you for the comment forgot about that, marked it as ready now 😄
As you mention putting the logic in python functions I realised that you mentioned this in the issue, I can refactor this and do it just need to have a look how to do such (should be easy enough I take)

[choldgraf]

In case it helps, check out here for where we do similar stuff w other functions:

pydata-sphinx-theme/pydata_sphinx_theme/__init__.py

Line 253 in f54466a

context["generate_nav_html"] = generate_nav_html

And then it is called in the template here

pydata-sphinx-theme/pydata_sphinx_theme/_templates/navbar-nav.html

Line 2 in f54466a

{{ generate_nav_html("navbar", maxdepth=1, collapse=True, includehidden=True, titles_only=True) }}

[FabioRosado]

Thank you @choldgraf for pointing me in the right direction. I've updated the PR with your suggestion, could I check with you if this is what you had in mind?

Initially, I tried to format the strings in a nicer way, but some were longer than 88 characters so some of them look a bit weird 😄

[choldgraf]

wanna just make these f-strings? like:

script = f"""
<script async ...>
  stuff
  {id}
  stuff
</script>

[FabioRosado]

Apologies for another push, I've missed this comment. That sounds good, the only issue is that the script will have a lot of white space on the final HTML page, but that should be alright

[choldgraf]

what role does kind play here? I don't see it used (if unused, can we just remove and use a single arg for id?

[FabioRosado]

Thank you for the comment, I was following the pattern when testing things and forgot to remove it. Your comment also made me realise that we don't really need kwargs here since we are passing the id only so thank you for that as well 😄

[choldgraf]

This looks great, thanks very much @FabioRosado :-)