Cartodb: Viewport in embed does not always respect viewport in Builder

[AbelVM]

Context

Trying to fit the view to a layer in a dashboard

Steps to Reproduce

• Any map, any layer
• "Fit to layer"
  
• Publish
• The viewport can be quite odd. Mainly because of zoom levels
  

Current Result

The view is zoomed out 1 step in embed when zoom level in Builder is <= 6

Expected result

Best fit of viewport, keeping center and same zoom as Builder, or best fit for BB

Browser and version

Any, but could it be related to screen aspect ratio?

cc @javitonino

[xavijam]

Well, we can save center + zoom, there is no problem. Are we sure of that?

[saleiva]

what are we storing now?

[javitonino]

The view bounds in Builder (longitude and latitude of the NE and SW corners of the screen). The idea being to keep the same viewport in screens of different sizes, even if it means changing zoom.

[saleiva]

Yes, I do think we will have to switch to zoom + center.

[xavijam]

👍 . We could store both things, bounds and center+zoom, and then decide what to use in the embed view, sounds good? It would be possible with the state model in backend, wouldn't it @CartoDB/backend? I've checked it just expects a json so...

[javitonino]

No problem adding it to state, as you said it's just a JSON and it is presented fully in embeds. No filtering or validation going on, so it should be ok.

I'd keep using map bounds, at least for much different screen sizes (mobile), otherwise you only see a very small portion of the map.

[xavijam]

I agree with @javitonino, but you decide.

[talos]

Agreed we need to resolve this, right now depending on if you're on retina/mobile/non-retina our recently published blog post has maps centered on totally different areas (sometimes not even over the USA).

[jgoizueta]

I see no perfect solution for this, the problem being that what should ideally preserved is some Area of Interest that the user has in mind, and preserving either the bounds or the center/zoom will produce unwanted results in general:

[xavijam]

Very nice explanation man! 💋

[saleiva]

let's go with c+z since it's what covers most of the corner cases

[AbelVM]

I used to use a hybrid solution for aspect-independent bbox:

• Get BBox
• Calc the "max square within bbox". I mean, cut the rectangle shaped BBox to get a squared one with the same center and the the side the same size as the shortest size of the original BBox
• Save this as your Builder BBox
  ...
• FIt bounds to this square in Embed

[jgoizueta]

It seems like reasonable compromise to me, should work for most AOI (unless it is too tall or too wide)

[xavijam]

Hey guys, after fixing this issue and this issue, I feel the experience will be totally different. Could you give us one day to fix, merge, deploy and then test it the behaviour is consistent and good enough? I'm pretty confident we will be happy, promised.

[xavijam]

In any case, we have to block it until we have those issues fixed.

[saleiva]

What are we doing with this then?

[AbelVM]

[alonsogarciapablo]

Bump!

The PR we have to solve https://github.com/CartoDB/cartodb/issues/11783 breaks this (a bit more) so I'm having to investigate how all this works behind the scenes. I'm going to describe my understanding of how this currently works:

Once the viz.json and map has been loaded (this is what we're changing in #11738), deep-insights.js reads the state from URL (or given opts) and:

1. If bounds are present in the state, it instructs cartodb.js to calculate and adjust the zoom level to the given bounds (via Map#setBounds). This is the method that performs the zoom calculation for some given bounds. I think this is the default behaviour for BUILDER since state now also includes bounds. This requires the map view to be ready because the new zoom level is directly related to the size of the map's container.

2. If bounds are not present in the state but zoom and center are present, it instructs cartodb.js to set some new zoom and center (via Map#setView). I guess this is the behaviour we had before bounds were stored in the state. There's no need to do this explicitly here. Instead, we could set the initial center + zoom here.

So now that this is a bit more broken (in our PR), I see two options:

1. Forget about trying to automagically adjust zoom for given bounds and render the map with center and zoom values from state (if present) for now. Many of you are suggesting this. We can then find a better way of doing this (eg: what @AbelVM suggested).
2. I can change things in my PR and leave things as they were, although we'll still have this issue.

@saleiva @noguerol this is a product decision so I guess you guys should decide how to go with this. Thanks! Of course, easiest thing for me at this point would be to go with 1.

[alonsogarciapablo]

cc: @donflopez ☝️

[saleiva]

I'd go with option 1 @alonsogarciapablo

I do think that user expect maintaining the center more than the "aspect ratio" of the map – and the behaviour is easier to explain and understand.

[alonsogarciapablo]

👍 Thanks @saleiva!

[nobuti]

In production.