Introduce `Container` model

[huong-li-nguyen]

Description

• Enables Container model as a new page component with these fields: title, components, layout
• Renames selected CSS classes (other ideas welcome, it's getting trickier now with all the components, containers and their nested elements 😄)
• Refactor previously private methods of Page as reusable functions
• Transform duplicated validators into reusable validators and apply to Page, Container and Form
• Refactor actions code to incorporate for Containers and nested Containers

Note: Tests and Docs will be done in a separate PR as this one is already quite big

• [x] Docs: https://github.com/mckinsey/vizro/pull/262
• [x] Tests: https://github.com/mckinsey/vizro/pull/265

Screenshot

Notice

• [x] 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.
  • 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.

[petar-qb]

I would like to hear your thoughts on added _model_manager and _page helper methods, hence this thread is created.

Should they be where they are now, or should they be moved somewhere else?

Do we also need to make newly added _page helper methods more generic like:

• _get_page_actions_chains -> _get_model_actions_chains
• _get_page_model_ids_with_figure -> _get_model_ids_with_figure

and move them to the _model_manager?

[l0uden]

@huong-li-nguyen

I've found old bug from the previous branch of tabs an containers where table in one container overwrites title of the second container

[l0uden]

@huong-li-nguyen , can we do smth with x-axis name conflicting with the legend items? it is present on the main screenshot of this PR with four charts in the container.

[huong-li-nguyen]

@huong-li-nguyen , can we do smth with x-axis name conflicting with the legend items? it is present on the main screenshot of this PR with four charts in the container.

This has to be manually set by the user by providing a custom Layout and providing a row_min_height. This then activates scrolling. The other option would be to use custom charts and just turn off the legend. But generally all solutions have to be done via the configuration. Are you using this example for your tests right now?

[huong-li-nguyen]

@huong-li-nguyen

I've found old bug from the previous branch of tabs an containers where table in one container overwrites title of the second container

Thanks! I'll take a look later

[l0uden]

@huong-li-nguyen , can we do smth with x-axis name conflicting with the legend items? it is present on the main screenshot of this PR with four charts in the container.

This has to be manually set by the user by providing a custom Layout and providing a row_min_height. This then activates scrolling. The other option would be to use custom charts and just turn off the legend. But generally all solutions have to be done via the configuration. Are you using this example for your tests right now?

I'm using my own, but it has the same problem. I will try to use first option

[antonymilne]

@maxschulz-COL @huong-li-nguyen @petar-qb this all gets a kind of pre-approval from me 💯 I left lots of comments but once those are resolved let me know and I'll re-review and chances are very high it will be an approval 😀 Just give me a shout if it's easier to discuss the comments in person or work through them together instead of getting too many long comment threads on github.

[huong-li-nguyen]

I still need to review the actions stuff, but here's a partial review so you're not waiting too long...

Generally this looks great I think! Some general questions:

1. Page.controls applies to all figures on the page, regardless of whether they're in a (possibly nested) container, right?
2. did you test the case where you have Page.components = [vm.Container(), vm.Graph()], so a container and a non-container as siblings?
3. are we saving the Container.controls field for a separate PR?
4. for naming, I'd suggest maybe using grid more since ultimately that's what these classes and functions are for. They don't make sense without a Layout.grid.

@antonymilne - regarding your questions:

1. Yes, that's the case
2. Yes, I've also added a test example here: https://github.com/mckinsey/vizro/pull/254/commits/8ccf044ffe6c16cda937d6435fdb575b5545251f
3. Container.controls will come later
4. What does that refer to again? 😄

@l0uden - in https://github.com/mckinsey/vizro/pull/254/commits/8ccf044ffe6c16cda937d6435fdb575b5545251f I've also added the example with a Table and a Container and there is no overlapping anymore. Can you double-check if that's the bug you've referred to? :)

[l0uden]

I still need to review the actions stuff, but here's a partial review so you're not waiting too long... Generally this looks great I think! Some general questions:

1. Page.controls applies to all figures on the page, regardless of whether they're in a (possibly nested) container, right?
2. did you test the case where you have Page.components = [vm.Container(), vm.Graph()], so a container and a non-container as siblings?
3. are we saving the Container.controls field for a separate PR?
4. for naming, I'd suggest maybe using grid more since ultimately that's what these classes and functions are for. They don't make sense without a Layout.grid.

@antonymilne - regarding your questions:

1. Yes, that's the case
2. Yes, I've also added a test example here: 8ccf044
3. Container.controls will come later
4. What does that refer to again? 😄

@l0uden - in 8ccf044 I've also added the example with a Table and a Container and there is no overlapping anymore. Can you double-check if that's the bug you've referred to? :)

No, the bug is still reproducing. To do it just put table into the separate container. In your example it's just a graph.

[huong-li-nguyen]

@l0uden - got it! I've updated the example and added a fix. Can you check again? :)

I'll probably take that fix out and create a separate PR on main, such that @maxschulz-COL can pull it later for his PR on the ag-grid to see how it looks like 👍