Improve dashboard title #228

antonymilne · 2023-12-19T00:36:52Z

Description

Small precursor to #185, mentioned as part of #176.

When Dashboard.title is specified we now include it in the page title in the dash registry. This makes no difference to anything on the screen other than the <title> tag and a meta tag. These are useful to get right because it means that the title of the page is much more meaningful in your browser window, when you try to bookmark a dashboard, and if you share a link of a Vizro dashboard that gets previewed e.g. on social media.

Screenshot

Checklist

I have not referenced individuals, products or companies in any commits, directly or indirectly
I have not added data or restricted code in any commits, directly or indirectly
I have updated the docstring of any public function/class/model changed
I have added tests to cover my changes (if applicable)

Types of changes

Docs/refactoring (non-breaking change which improves codebase)
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Notice

I acknowledge and agree that, by checking this box and clicking "Submit Pull Request":
- I submit this contribution under the Apache 2.0 license and represent that I am entitled to do so on behalf of myself, my employer, or relevant third parties, as applicable.
- I certify that (a) this contribution is my original creation and / or (b) to the extent it is not my original creation, I am authorized to submit this contribution on behalf of the original creator(s) or their licensees.
- I certify that the use of this contribution as authorized by the Apache 2.0 license does not violate the intellectual property rights of anyone else.

vizro-core/changelog.d/20231218_204302_antony.milne_improve_dashboard_title.md

vizro-core/examples/default/app.py

antonymilne · 2023-12-19T00:42:24Z

vizro-core/src/vizro/models/_dashboard.py

-                module=page.id, name=page.title, path=path, order=order, layout=partial(self._make_page_layout, page)
+                module=page.id,
+                name=page.title,
+                title=f"{self.title}: {page.title}" if self.title else page.title,


This one line is the only functional change in the PR.

I guess if people do want to overwrite it, they would overwrite the pre-build of the model here?

I was actually wondering how this works compared to the dash.app.title. If you set the dash.app.title, I noticed that it appears for a second and then it gets overwritten by anything defined in dash.register_page.

This might be worth mentioning in the docs if people try to overwrite it via the dash.app.title. This wouldn't actually work, because in the end it gets overwritten by the code here again. But I think this is also how it works in a pure Dash app?

app = Vizro(assets_folder="../assets").build(dashboard) app.title = "My title" app.run()

This is indeed how it works in a pure Dash app also:

Dash(title="X") corresponds to the property dash.app.title and sets the initial value of <title>X</title>

but this only appears very briefly when you're using Dash pages, because it gets overwritten straight away by dash.page_registry[page.id]["title"]

after this, the original app title=X has no effect on anything

So for Vizro users where Dash pages is always used, the title argument basically has no effect. @AnnMarieW please do correct me if I've missed something here.

We have possibly added to the confusion here because we have our own Dashboard.title property, and it's not obvious to a user what effect that has on anything. I considered before whether we should do dash.app.title = dashboard.title in Vizro.build() but decided it was basically pointless because, like you say, it just gets overwritten straight away. I think it would be a very small improvement though, since that way you wouldn't see the "Dash" title at all, so let me add it in here.

Change made in c00fe4e.

vizro-core/tests/unit/vizro/conftest.py

vizro-core/tests/unit/vizro/models/test_dashboard.py

Signed-off-by: Antony Milne <49395058+antonymilne@users.noreply.github.com>

antonymilne · 2023-12-19T00:53:34Z

@AnnMarieW: I wanted to add you as a reviewer for this but for some reason can't do so - maybe some limitation of our github org settings I guess 🤔

Are you able to add yourself as reviewer?

There's two reasons I wanted to add you anyway:

so you can check I haven't done anything stupid
I was thinking about the tests in your PR and decided our current tests weren't quite right so rewrote them

I think we should aim to get this merged ASAP before #185 so you can base your tests on the "new" style.

AnnMarieW · 2023-12-19T03:53:33Z

@antonymilne I might have to be invited as a collaborator or a member before I can be added as a reviewer, but I think tagging me here works too :-)

I like the new style of the tests and will wait until this is merged before writing the tests in my PR. Thanks for the help!

I also like the new format for the title for the browser and the meta tags. Are you planning on updating the docs in a different PR?

nadijagraca

I love the new title format. 🚀

huong-li-nguyen

Looks great to me 👍

If we can just mention in the docs how the dash.app.title and dashboard.title/page.title overwriting works - I think that would help. I wasn't e.g. sure how it would actually work until I tested it out and saw that the dash.app.title is always overwritten by the title defined in the dash.page_registry. I guess that's the default behaviour coming from dash?

vizro-core/changelog.d/20231218_204302_antony.milne_improve_dashboard_title.md

huong-li-nguyen · 2023-12-19T09:06:22Z

vizro-core/examples/default/app.py

@@ -545,6 +545,7 @@ def create_home_page():


 dashboard = vm.Dashboard(
+    title="Vizro demo",


You probably just left this in for demo purposes, but I would remove it if this is close to being merged. It's not optimal in terms of design if there are controls/navigation on the page, as we still have this open discussion of where this might go.

Looking at this, I prefer the upper header container we discussed the other day even more 😄

Actually I left it here so that we would hopefully apply the same change to vizro.mckinsey.com next time we update the demo, but let me remove it if we're not yet happy about the visual design.

The reason I think it would be nice to include sooner rather than later is because every time someone shares a link to vizro.mckinsey.com the social media preview will have a slightly strange label (like just "Homepage") - it would be nicer if it instead said "Vizro demo: Homepage" or similar, to make it clear what the link being shared actually is.

If we don't work out the page layout/visual design soon then possible alternatives might be:

we remove the title from the dashboard layout altogether from _make_page_layout for now

we remove the title from the dashboard layout just by customising our demo app

So I'll revert the change here and leave it up to you to decide out what's the best solution here depending on how long the reworking of the page layout takes 🙂 It's not a huge thing but I don't think we should wait too long (say > 1 month) to get an improved shareable link to our demo app just because we're blocked on where to put the title on screen.

we remove the title from the dashboard layout altogether from _make_page_layout for now
I think this would be a breaking change right? Or are we still fine saying that any visual changes are not considered breaking changes? 😄 I would be fine with this one to be honest - if people reach out, then we at least know people have been using a dashboard title and need it 😅

we remove the title from the dashboard layout just by customising our demo app
As a quick-fix, this one seems safer. The demo app would be a bit out of sync for a while with the example app here, but I think it's fine as we manage it.

What do you prefer? 1) I could embed in my current PR on layout arrangements and 2) I can create in another PR in the demo-app repository

Reverted in 5cef34a.

I wouldn't consider removing the title from the layout a breaking change since it's just something visual and no one's code will break in any way. Admittedly it's kind of a drastic one rather than just tweaking styling, but I'm completely fine with it and, like you say, if people ask where their title went then it's a good signal they want it there.

Which solution we go for depends to me on how close we are to resolving the question of where the title goes and how upsetting people find the current placement:

if we're really not happy with how the dashboard title appears at the moment and it feels a long way off resolving let's go for option 1

if we think it's ok currently but still don't want it on our demo app then let's go for 2

if we think the current dashboard title is already completely fine then let's just leave it as it is and add a title to the example app

Personally I'm completely fine with the current visual appearance, which is why I just went for option 3 to begin with in this PR. But you have a better idea of how upsetting people with more design sensibilities find that and where future changes to layout might take us here, so completely up to you which option to choose 🙂

That makes a lot of sense!

I think the position of the dashboard title will likely change given that it's taking too much horizontal space if there are no controls/navigations. Given that J. is also out, I'll go for 2) as I think it's the one he would feel less reluctant to implement 😄

I add it to my tickets and will update the demo accordingly after we've done release 0.1.8 👍

Cool, thank you! Sounds good to me.

I didn't actually realise how weird it looks like when there's no controls/navigation until I just looked again now. You're right that doesn't look good at all, so I take back my statement above that option 3 was ok by me. Just in case you thought I had no design sensibilities at all 😀

P.S. hopefully the demo update would be as simple as dash.layout["dashboard_title_outer"].hidden = True or similar rather than needing any customisation of _make_page_layout with a custom Dashboard model.

huong-li-nguyen · 2023-12-19T09:12:21Z

vizro-core/src/vizro/models/_dashboard.py

-                module=page.id, name=page.title, path=path, order=order, layout=partial(self._make_page_layout, page)
+                module=page.id,
+                name=page.title,
+                title=f"{self.title}: {page.title}" if self.title else page.title,


I guess if people do want to overwrite it, they would overwrite the pre-build of the model here?

I was actually wondering how this works compared to the dash.app.title. If you set the dash.app.title, I noticed that it appears for a second and then it gets overwritten by anything defined in dash.register_page.

This might be worth mentioning in the docs if people try to overwrite it via the dash.app.title. This wouldn't actually work, because in the end it gets overwritten by the code here again. But I think this is also how it works in a pure Dash app?

app = Vizro(assets_folder="../assets").build(dashboard) app.title = "My title" app.run()

vizro-core/tests/unit/vizro/models/test_dashboard.py

antonymilne · 2023-12-19T10:27:25Z

@antonymilne I might have to be invited as a collaborator or a member before I can be added as a reviewer, but I think tagging me here works too :-)

I like the new style of the tests and will wait until this is merged before writing the tests in my PR. Thanks for the help!

I also like the new format for the title for the browser and the meta tags. Are you planning on updating the docs in a different PR?

I've just updated the docs in 7f981fb.

When we write docs for your meta tag image and description pieces, I think it would belong in pages.md and then let's link to it from my text that mentions meta elements here in dashboard.md.

I've changed a github setting that should mean that all users can now review PRs, but I still don't seem to be able to add you here. Maybe it will work in future PRs though, let's see.

vizro-core/examples/default/app.py

Signed-off-by: Antony Milne <49395058+antonymilne@users.noreply.github.com>

antonymilne added 2 commits December 19, 2023 00:36

Improve dashboard title

4def8ed

Lint and changelog

cfc3cf7

antonymilne commented Dec 19, 2023

View reviewed changes

antonymilne and others added 2 commits December 19, 2023 00:45

Update vizro-core/examples/default/app.py

e11f3b6

Signed-off-by: Antony Milne <49395058+antonymilne@users.noreply.github.com>

Update example

20dcd1a

antonymilne marked this pull request as ready for review December 19, 2023 00:51

antonymilne requested review from Joseph-Perkins, huong-li-nguyen, maxschulz-COL and lingyielia as code owners December 19, 2023 00:51

antonymilne requested a review from nadijagraca December 19, 2023 00:52

antonymilne mentioned this pull request Dec 19, 2023

Updating Meta Tags #185

Merged

9 tasks

nadijagraca approved these changes Dec 19, 2023

View reviewed changes

huong-li-nguyen approved these changes Dec 19, 2023

View reviewed changes

antonymilne added 3 commits December 19, 2023 09:55

Revert example change

5cef34a

Update app title if dashboard.title set

c00fe4e

Update docs

7f981fb

antonymilne and others added 2 commits December 19, 2023 10:54

Update JSON schema comment

b5ea319

Merge branch 'main' into feature/improve-dashboard-title

708bb8e

antonymilne enabled auto-merge (squash) December 19, 2023 10:55

antonymilne commented Dec 19, 2023

View reviewed changes

vizro-core/examples/default/app.py Outdated Show resolved Hide resolved

antonymilne added 2 commits December 19, 2023 11:44

Update vizro-core/examples/default/app.py

638f0b0

Signed-off-by: Antony Milne <49395058+antonymilne@users.noreply.github.com>

Merge branch 'main' into feature/improve-dashboard-title

403097a

antonymilne merged commit 2212992 into main Dec 19, 2023
37 checks passed

antonymilne deleted the feature/improve-dashboard-title branch December 19, 2023 13:00

antonymilne mentioned this pull request May 21, 2024

Updating Meta Tags #176

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve dashboard title #228

Improve dashboard title #228

antonymilne commented Dec 19, 2023 •

edited

Loading

antonymilne Dec 19, 2023

huong-li-nguyen Dec 19, 2023

antonymilne Dec 19, 2023

antonymilne Dec 19, 2023

antonymilne commented Dec 19, 2023

AnnMarieW commented Dec 19, 2023 •

edited

Loading

nadijagraca left a comment

huong-li-nguyen left a comment •

edited

Loading

huong-li-nguyen Dec 19, 2023

antonymilne Dec 19, 2023

huong-li-nguyen Dec 19, 2023 •

edited

Loading

antonymilne Dec 19, 2023

antonymilne Dec 19, 2023

huong-li-nguyen Dec 19, 2023

antonymilne Dec 19, 2023

antonymilne Dec 19, 2023

huong-li-nguyen Dec 19, 2023

antonymilne commented Dec 19, 2023

		@@ -545,6 +545,7 @@ def create_home_page():


		dashboard = vm.Dashboard(
		title="Vizro demo",

Improve dashboard title #228

Improve dashboard title #228

Conversation

antonymilne commented Dec 19, 2023 • edited Loading

Description

Screenshot

Checklist

Types of changes

Notice

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antonymilne commented Dec 19, 2023

AnnMarieW commented Dec 19, 2023 • edited Loading

nadijagraca left a comment

Choose a reason for hiding this comment

huong-li-nguyen left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huong-li-nguyen Dec 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antonymilne commented Dec 19, 2023

antonymilne commented Dec 19, 2023 •

edited

Loading

AnnMarieW commented Dec 19, 2023 •

edited

Loading

huong-li-nguyen left a comment •

edited

Loading

huong-li-nguyen Dec 19, 2023 •

edited

Loading