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

Update meta tag information #99

Merged
merged 5 commits into from
May 10, 2022
Merged

Update meta tag information #99

merged 5 commits into from
May 10, 2022

Conversation

bradley-erickson
Copy link
Contributor

This is not fully tested, I do not have access to a domain I can test on to ensure that this works. This will need to be deployed to a domain in order to test properly. We can use https://metatags.io/ to ensure things are showing up properly after deploying.

I'm am not 100% sure how we should handle the image url and url information.

The changes can be described as the following:

  • twitter:card will be set to "summary_large_image", this could perhaps be the default and we allow the user to choose the proper card type; however, I think this is the most often used Twitter card for sharing websites.
  • image_url is a new parameter to pass into each page. I figure that some people might enjoy accessing the image locally (through the assets folder using image), but we need to point to a proper image location for sharing.
  • url is used to specify the full path of the deployed instance instead of the relative path.

@AnnMarieW
Copy link
Collaborator

Hey @bradley-erickson Thanks for the PR!

The og card for sharing on linked-in and the forum works - for both the URL and the image, so we need to make sure the changes are only for the twitter card.

I think it would be great if we could handle this without requiring people to enter more data in dash.register_page()

For the url, I think this will work - it should capture the full url including any pathname prefix:

<meta property="twitter:url" content="{flask.request.url}">

For the image, if it requires a full url, we can get the base path with request.url_root,

<meta property="twitter:image" content="{image_url}">
 image_url = "".join([flask.request.url_root, image.lstrip("/")])

If you would like to make the update, I could deploy a tarball based on this PR, and see if it works.

@bradley-erickson
Copy link
Contributor Author

Knowing that sharing works for the og card makes me suspect that it might work for the Twitter cards, and the issue was only in the twitter:card attribute. I'd have tested this more if I had the easy capability to.

@AnnMarieW
Copy link
Collaborator

AnnMarieW commented May 7, 2022

OK, I'll try it with just changing the twitter card and seeing if it works for the image.

update:
This site has the changes: https://dashlabs.pythonanywhere.com/

@AnnMarieW
Copy link
Collaborator

Just need to add the new image_url to the page_registry at line 185:

page.update(
        image=(image if image is not None else _infer_image(module)),
        supplied_image=image,
        image_url=image_url,        
    )

@AnnMarieW
Copy link
Collaborator

Hey @bradley-erickson This looks fantastic and the umage_url is a great new feature. Thanks for your contribution!

@T4rk1n This is ready for a final review:
Closes issue #98

  • fixes a bug in the twitter card meta tags.
  • adds image_url property to dash.page_registry This gives the option of using a link for the image in the meta tags rather than an image file in the app's assets folder.

The changes are deployed in the dash-labs online docs, and we have verified that the Twitter card is fixed. The formatting, image and URL are all now correct. After it's merged I can do another release ad make an announcement on the forum.

Copy link
Contributor

@T4rk1n T4rk1n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💃 Looks good, verified the bird image appears in the twitter card validator

@AnnMarieW AnnMarieW merged commit aa792aa into plotly:main May 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants