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

Updating Meta Tags #176

Closed
1 task done
AnnMarieW opened this issue Nov 17, 2023 · 8 comments · Fixed by #185
Closed
1 task done

Updating Meta Tags #176

AnnMarieW opened this issue Nov 17, 2023 · 8 comments · Fixed by #185
Assignees
Labels
Community Issue/PR opened by the open-source community Issue: Feature Request 🤓 Issue/PR contains a feature request or is based on a feature request

Comments

@AnnMarieW
Copy link
Contributor

AnnMarieW commented Nov 17, 2023

What's the problem this feature will solve?

One of the cool features of Dash Pages is that it’s easy to update the meta tags for each page. This is good for SEO and creating nice looking preview cards when sharing a link on social media.

Currently with the Vizro demo app, only the title is included in the meta tags. Here's what the link preview looks like:
vizro_on_forum

However, Dash Pages will update the meta tags with an image and description if supplied. For example, see the Dash Example Index. It has over 100 pages, each with it's own title, description and image:

vizro_vs_dash_example_index

Describe the solution you'd like

It's easy to enable this feature in Vizro. Simple include (an optional) description, image, and image_url parameter in the Page.

I could do a pull request if you would like this feature enabled. I got it working locally. You can't see what the link looks like until the app is deployed , but when inspecting the browser, you can see that the meta tags now include an image and description.

vizro_updated_meta

Alternative Solutions

No

Additional context

None at this time.

Code of Conduct

@AnnMarieW AnnMarieW added Issue: Feature Request 🤓 Issue/PR contains a feature request or is based on a feature request Status: Needs triage 🔍 Issue/PR needs triaging labels Nov 17, 2023
@antonymilne
Copy link
Collaborator

Hi @AnnMarieW, thanks very much for the suggestion and the offer to raise a PR 🙏 It's been on my list for a while to check what other things we could set on dash.register_page in case we were missing any nice tricks like this, so I love the idea!

I do have a few hesitations about adding this feature right away though:

  • we haven't yet enabled functionality to insert a dashboard-level logo (it's on our backlog though), and I'd like to make sure that however we handle that would be consistent with page-level image information. Related, Enable NavBar as selector for Navigation #70 is our first foray into using navbar icons and we have opted to use Google material icons for that, so I'm not sure what the current thinking is on logos
  • a page-level description makes a lot of sense to me outside of meta tags anyway - I wonder if we could also use the information as part of the page content and/or homepage navigation cards themselves. I'm not sure whether this has been considered before or if we have designs for it though
  • having page-level description and image feels quite advanced when we don't currently enable app-level options for these (related to question 1 below...)

So let me check with our project manager @Joseph-Perkins where things stand on the above and get back to you!

A couple of questions for you if you don't mind!

  1. customising these meta tags is very nice, and I really like the _infer_image functionality that automates discovery of this sort of thing. However, it feels quite advanced to be able to set these things on a per-page basis when (as far I can tell) there's no equivalent way to achieve the more basic requirement of specifying the information at a global app-level. You can enter meta_tags in the Dash instantiation, but it's all manual rather than looking for logo.png etc. automatically. Why is this functionality is specific to Dash pages in the first place? Of course since we always use Dash pages, we could achieve app-level tags by exposing Dashboard.description and then using the same description for every page in our dash.register_page call. But I'm wondering if there's a good reason that you shouldn't easily be able to provide an app-level description etc. already in pure Dash?
  2. something I nearly changed before and then decided again for reasons that I can't now remember... I was going to set the page title to something like title = f"{dashboard.title}: {page.title}" if dashboard.title else page.title so that instead of the page name being Homepage it would be My dashboard name: Homepage etc. Does this sound like a good idea to you?

@AnnMarieW
Copy link
Contributor Author

Hi @antonymilne and thanks for your response.

The icons, logos, images and descriptions used in the layout may not be ideal for the meta tags. The dash.page_registry allows for arbitrary data to be added, so it's possible to include that page level data there. However it's best to keep keywords description, image and image_url reserved for the meta tags because Dash Pages automatically uses that data to create the meta tags. Also, Pages will use certain image files from the assets folder for the meta tags, so that feature should either be included in the Vizro docs or disabled so there are no surprises.

Related to #70 -- in the next release of Dash Bootstrap Components, you will be able to put icons (or any Dash component) in the Accordion title. This means you won't need the limitation of including icons only in the navbar. 🎉 see dbc PR # 988

Having unique meta tags for every page is an advanced (and very cool!) feature, however you can use the same image and description for every page if you like. Simply include an image file named "logo" or "app" in the assets folder. To add the same description to every page, you can loop through the dash.page_registry and add it to each page with the keyword "description".

There is no good reason why app.description and app.image is not included in Dash without using Pages. That's a great feature request! Feel free to open an issue in the Dash repo.

Re the title = f"{dashboard.title}: {page.title}" I like it! That's a very nice default.

I have some other general Vizro design questions, but they aren't "issues". Is it OK to ask them here, or do you prefer some other channel?

@antonymilne
Copy link
Collaborator

Thanks for all the answers @AnnMarieW! That's very nice to know about the dbc.Accordion also 👌

I had a good chat with @Joseph-Perkins about all this yesterday. What you say makes a lot of sense and we're keen to go ahead with enabling the functionality, but I'd just like to give a bit more thought about how exactly we handle it in a way that plays nicely with our existing plans to enable a dashboard logo. So please hold off on the PR just for now and I will get back to you very soon if that's ok! 🙏

Very happy to discuss Vizro design questions and anything else more generally - let me send you a message on the plotly forums and we can figure out the best way to be in touch.

@AnnMarieW
Copy link
Contributor Author

I'm happy to help any time it fits with your development plan. :-)

@antonymilne
Copy link
Collaborator

antonymilne commented Nov 23, 2023

I've thought about this all some more and here's what I think we should do 🙂

  • add the field description: Optional[str] to the Page model
  • no need for image or image_url fields in Page - the way for users to enable these would be for them to put include an image of the right name (assets/<page.id>.<extension>) so it gets picked up automatically
  • add some appropriate image assets/app.<extension> to our examples so that the whole thing gets a nice image when shared (will talk with @Joseph-Perkins to figure out what this image would be)
  • add documentation on all the above and (can just point to Dash documentation where useful, but would be nice to outline the most likely scenarios people want to achieve e.g. just one app.png image vs. individual page-level images)
  • no new fields in Dashboard for now at least

So if you'd like to raise a PR then please go for it @AnnMarieW! I'll be more than happy to support, answer questions, help with tests, review, etc. - whatever you like. No need to put all of it in one PR if you'd prefer to do the docs separately or just leave that to someone else and make only the code changes - completely up to you.

If you'd like to understand more context around why I suggest the above, I can definitely explain more - it's all to do with the architecture of Vizro and what level the interface should be. This currently isn't spelt out very clearly anywhere other than my head so I appreciate it might be a bit mysterious without further explanation 😅


Known limitations that are all deliberate and could potentially be relaxed in future if there's demand for it, but not now:

  • description cannot be a function, just a str
  • image and image_url cannot be specified manually, just picked up automatically
  • similarly for path to a logo that's used in the page

Known limitation that I would like to lift, but not a big problem: images are only picked up if they're at the root of the assets folder. This conflicts with our current suggested structure that images live inside assets/images. Possible solutions:

  • modify dash._page._infer_image to look recursively through the assets folder (@AnnMarieW can hopefully comment on whether this would be a desired solution or not)
  • we roll our own _infer_image function in Vizro that does the same but looks recursively
  • we revise our suggested structure to put all images in the root of assets
  • we just live with the slight inconsistency that page images must live in the root of assets while we put others inside a subfolder - this will be the situation to begin with at least, and it's not a terrible problem

Near future changes:

  • @antonymilne to modify page.title as suggested above unless he can remember why he didn't do this before...
  • @nadijagraca will add functionality to include a logo in the layout itself. This should follow exactly the same logic as dash._page._infer_image but only look for a file called logo. No need for a logo field in Dashboard
  • consider how/whether to handle the case that only some of the pages have a description. Should we have a description field in Dashboard which is the default for Page.description? Or should all page descriptions that aren't filled in just take the homepage description? Or should we rely on a Dash implementation of app-level description (either through feature request or through supplying meta_tags argument to Dash)?

In fact, I just realised we'll need a function like dash._page._infer_image to put the logo into our layout anyway. So regardless of the above "Known limitation that I would like to lift", we'll need to roll our own anyway unless that function is made public in Dash.

Not so near future changes:

  • we enable automatic generation of a navigation homepage that looks a bit like the one on https://vizro.mckinsey.com/ using the page.description, image, etc. (This was on the backlog already)
  • we make something clever to automatically take screenshots of the different dashboard pages so that users don't need to do that manually themselves at all. In addition to the navigation homepage and meta tags, this might also be used e.g. in a tooltip when hovering over navigation? I have some ideas on how to achieve this and it sounds like a fun little project, but not super easy or high priority

@AnnMarieW
Copy link
Contributor Author

I'd be delighted to do the PR for both the code changes and the docs. 🙂

modify dash._page._infer_image to look recursively through the assets folder

This would be the ideal solution, but I'm concerned that it would be considered a breaking change. I'll check with Plotly and if they approve, I could do a PR in Dash. Otherwise, I could add it to Vizro if you like.

we make something clever to automatically take screenshots of the different dashboard pages so that users don't need to do that manually themselves at all

I have a utility that takes a snapshot of each page using Selenium. If that's what you have in mind, I can share that with you when you're ready for a fun little project.

There is now a feature request in Dash for app level description and image to make it easier to create Meta Tags.

@antonymilne
Copy link
Collaborator

I'd be delighted to do the PR for both the code changes and the docs. 🙂

Amazing, thank you! Our contributing guidelines is pretty thorough but may be slightly out of date in places. I'm not sure anyone external has tried following it in detail before, so I'll be very interested in any feedback you have on any aspect the contribution process.

One thing that's not mentioned there that will be useful if you're modifying docs: hatch run docs:serve will do a hot-reloading build of the rendered docs. Other than that, hatch run lint is the main command you'll find you need to pass CI.

modify dash._page._infer_image to look recursively through the assets folder

This would be the ideal solution, but I'm concerned that it would be considered a breaking change. I'll check with Plotly and if they approve, I could do a PR in Dash. Otherwise, I could add it to Vizro if you like.

Cool, let me know what they say. Though I realised that we'll need to write our own version of _infer_image anyway to find the logo automatically to put in the layout, and it's a small and simple function that's easy for us to copy over. So to be honest it's probably not worth making the change on the Dash side on second thoughts, since they would also need to make the function public API which probably doesn't make sense just for our benefit. So no pressure to try and change anything on the Dash side - I'd only ask about adding it there if you think it would be a general enhancement beyond just Vizro's needs.

I have a utility that takes a snapshot of each page using Selenium. If that's what you have in mind, I can share that with you when you're ready for a fun little project.

Yeah, that's the sort of thing I had in mind! We have something similar for running QA tests though I don't know the details of it.

Ideally I think we'd take a screenshot of just the main content of the page (so not including the navigation), maybe doing something like this? I'm not very familiar with Selenium but my impression is that it might be a bit too heavyweight/awkward for general users to setup (installing chromedriver doesn't seem trivial for a start, but it's very possible that I just don't know what I'm doing). Some possible alternatives (never used any of them) might be:

There is now a feature request in Dash for app level description and image to make it easier to create Meta Tags.

Nice! Let me leave a comment there.

@AnnMarieW
Copy link
Contributor Author

@antonymilne

I opened an issue in Dash for the recursive search for the image files. While it may not be considered a breaking change, I'm not sure if this should be added to Pages.

There are several options that could work well with Vizro. Specifying the image to use for the meta tags in the dash.page_registry would probably be the easiest solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community Issue/PR opened by the open-source community Issue: Feature Request 🤓 Issue/PR contains a feature request or is based on a feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants