Move layout creation to Dashboard

[huong-li-nguyen]

Description

• Move layout creation to Dashboard
• For more context see: https://github.com/McK-Internal/vizro-internal/issues/20

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.

[antonymilne]

This is a huge improvement I think! 💯 I'm much happier with this now, thank you for making the change!

Just some very small comments but it looks great.

Also while I remember - we should discuss hidden Divs briefly.

I think we should close the original "Take title out of dashboard" ticket and create a new one for the grid-template-area which has all the relevant information there just to be clear where things now stand 🙂

[huong-li-nguyen]

Had to reduce as strangely this fails on the unit tests for 3.11 only: https://github.com/mckinsey/vizro/actions/runs/6718518053/job/18258559080?pr=142

We have a proper ticket to investigate further here:
https://github.com/McK-Internal/vizro-internal/issues/43

[maxschulz-COL]

For transparency:

• the reason this fails for python 3.11 and not others is not related to this PR: the coverage for different version is different also in previous checks (the reason seems to be the kedro integrations code, which skips some tests depending on the version)
• this means that this PR decreased the overall coverage just enough to push some tests below the threshold
• the culprit seems to lines 101-128 in _dashboard.py

Let's investigate this further in another PR, just wanted to clarify for the person that looks at this

[antonymilne]

Kedro 0.18.13 onwards support Python 3.11 so we should be able to remove our special skipping of kedro in that environment now anyway.

[huong-li-nguyen]

Created a new issue for this here: https://github.com/McK-Internal/vizro-internal/issues/363

[maxschulz-COL]

Managed to look at it all now! I really like it, and it's indeed a huge improvement.

I have been thinking: given that our coverage decreased with this PR, e.g. for lines 101-121 of _dashboard.py, could this be that it is linked to the fact that we are using partial there? And hence the _make_page_layout function is actually never called in any unit tests!

If that's the case, then at least we know where to fix it - but given that time is of the essence I would not focus on it more here

[huong-li-nguyen]

Managed to look at it all now! I really like it, and it's indeed a huge improvement.

I have been thinking: given that our coverage decreased with this PR, e.g. for lines 101-121 of _dashboard.py, could this be that it is linked to the fact that we are using partial there? And hence the _make_page_layout function is actually never called in any unit tests!

If that's the case, then at least we know where to fix it - but given that time is of the essence I would not focus on it more here

That could be it! We've linked to this PR in the relevant issue already, so this can be taken as reference