Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possibility to save all map layers at the same time #4251

Conversation

xavijam
Copy link
Contributor

@xavijam xavijam commented Jul 1, 2015

Basically we have a problem when we need to save the state of all layers those belong to a visualization.
We used to sent as many PUTs as layers we have in the visualization. From now on, we want to just make a request with the needed info.

Fixes #4016.
@juanignaciosl

@xavijam
Copy link
Contributor Author

xavijam commented Jul 1, 2015

So, with the current changes the request works correctly but:

  • Map is not refreshed.
  • Layers order aren't being render properly, mainly because of this. Seems like we need to sort layers or change that part of code and get 'order' attribute instead of getting the collection position.

cc @alonsogarciapablo @javisantana

@Cartofante
Copy link
Collaborator

Frontend tests were OK 👍 (details)

@Cartofante
Copy link
Collaborator

Frontend tests were OK 👍 (details)

@alonsogarciapablo
Copy link
Contributor

Hey! What's the status of this PR? @xavijam are you planning to do some more work here? (I'm asking because of the two caveats that you mentioned in your comment). This is somehow related to this PR, so it'd be nice to QA everything at the same time.

}
}
);
this.save(null, opts);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since save is also a one-liner, we could move its code here instead of adding a new public method.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally agree, but we still have the problem with the "invalidation" of the tiles when save is completed.

@ztephm
Copy link

ztephm commented Jul 15, 2015

Hi @xavijam @alonsogarciapablo @Cartofante , this issue is happening still for S/B 5831712

Think you guys already got this part, but:

Expected:

  • drag top layer to bottom of layer stack in Map View
  • data layers in map re-order according to new layer order

Happening (in Chrome 43.0.2357.134):

  • drag top layer to bottom of layer stack in Map View
  • data layers are not re-ordering
  • happening 1st/any time it's re-ordered, not just 2nd

Replicated here:

  1. Add line layer
  2. Add point layer
  3. Add polygon layer
  4. Drag polygon layer to bottom

Polygon layer shows at bottom of Map View layer stack but polygons are still on top of other line and point layers:
http://cdb.io/1I3nhYG

screen shot 2015-07-15 at 12 41 20 pm

@alonsogarciapablo alonsogarciapablo self-assigned this Jul 16, 2015
@Cartofante
Copy link
Collaborator

Frontend tests were OK 👍 (details)

@Cartofante
Copy link
Collaborator

Frontend tests were OK 👍 (details)

@alonsogarciapablo
Copy link
Contributor

I think this is now ready and working! 😌

@xavijam Could you PTAL? Thanks!

@xavijam
Copy link
Contributor Author

xavijam commented Jul 17, 2015

Looks great man :)

@Cartofante
Copy link
Collaborator

Frontend tests were OK 👍 (details)

@Cartofante
Copy link
Collaborator

Frontend tests were OK 👍 (details)

alonsogarciapablo added a commit that referenced this pull request Jul 20, 2015
…p_layers_at_the_same_time

Possibility to save all map layers at the same time
@alonsogarciapablo alonsogarciapablo merged commit 8fbf49f into master Jul 20, 2015
@alonsogarciapablo alonsogarciapablo deleted the 4016-Possibility_to_save_all_map_layers_at_the_same_time branch July 20, 2015 08:52
@xavijam
Copy link
Contributor Author

xavijam commented Jul 20, 2015

\o/ :dancer: :grapes:

@Cartofante
Copy link
Collaborator

Frontend tests were OK 👍 (details)

@alonsogarciapablo
Copy link
Contributor

Deployed and working 👍. Great job guys!

@juanignaciosl
Copy link
Contributor

Yahoo! :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants