include view definitions for a layercharts containing a repeat + toplevel selection parameter

[mattijn]

Fix #2849.

• In the LayerChart the params of the subcharts are combined, see: https://github.com/altair-viz/altair/blob/master/altair/vegalite/v5/api.py#L2978
• Currently the view for these params is populated here: https://github.com/altair-viz/altair/blob/master/altair/vegalite/v5/api.py#L3272. Around here is a loop on the subcharts, as far as I can see, they are not aware of being in a RepeatChart.
• But once we reach the RepeatChart, we are aware that it is a LayerChart, see: https://github.com/altair-viz/altair/blob/master/altair/vegalite/v5/api.py#L2664.
• At this place we also define the view names for the repeat. We can adjust the code here to accommodate for the required behavior.

With this PR we include the spec as argument in the function _repeat_names() and _extend_view_name(). If the instance of spec is Chart, the code functions as usual, but when spec is an instance of LayerChart we now also will enter the function _repeat_names(). And within the extend_view_name() the view_<no> should be placed at the end of the view-name instead (vs start for a non-layered repeat chart).

I included the interactive crossfilter as test, and tested it locally.

I think this works as expected, agree @ChristopherDavisUCI?

[ChristopherDavisUCI]

Great @mattijn, thanks! I tried this out and looked over the code and it all makes sense.

Are parameters now working fully as desired, or are there still some open tasks on the Vega-Lite side (or the Altair side) remaining?

[mattijn]

Thanks for the review. I will merge this in then. I think we are feature complete for another rc2 release, but I'll have to double check.

[binste]

Thanks for the review. I will merge this in then. I think we are feature complete for another rc2 release, but I'll have to double check.

Great to hear! I think it would be nice if we can merge #3009 in for the rc2 but that's the only outstanding issue/PR I'm aware of.

[mattijn]

You are right @binste, we should try to include #3009 as well, and maybe #2867 too.

[mattijn]

I had a test bank of different type of compound charts with interactivity included defined, see https://gist.github.com/mattijn/ddc294572cbb0191f9c213fe7afd995e.
All of them are now working properly including the interactive elements (zooming, clicking, hovering etc).