mapgl icon indicating copy to clipboard operation
mapgl copied to clipboard
verified publisher icon walkerke

Convert "layer" argument to "layer_id" in Shiny functions

Open mhpob opened this issue 9 months ago • 2 comments

Response to #86.

There are a bunch of misc. Air formatting changes in there (sorry!), so here are the pertinent changes for your review:

  • Convert layer argument to layer_id and add dots (incl. to roxygen comments)
    • https://github.com/mhpob/mapgl/blob/e96a53166d86dc916e4b22d8547b7e27f8ecf238/R/shiny.R#L160
    • https://github.com/mhpob/mapgl/blob/e96a53166d86dc916e4b22d8547b7e27f8ecf238/R/shiny.R#L225
    • https://github.com/mhpob/mapgl/blob/e96a53166d86dc916e4b22d8547b7e27f8ecf238/R/shiny.R#L451
    • https://github.com/mhpob/mapgl/blob/e96a53166d86dc916e4b22d8547b7e27f8ecf238/R/shiny.R#L510
  • If layer_id is missing, check for layer argument in dots. For compatibility with code written in previous versions where the argument was explicitly used.
    • https://github.com/mhpob/mapgl/blob/e96a53166d86dc916e4b22d8547b7e27f8ecf238/R/shiny.R#L162
    • https://github.com/mhpob/mapgl/blob/e96a53166d86dc916e4b22d8547b7e27f8ecf238/R/shiny.R#L227
    • https://github.com/mhpob/mapgl/blob/e96a53166d86dc916e4b22d8547b7e27f8ecf238/R/shiny.R#L453
    • https://github.com/mhpob/mapgl/blob/e96a53166d86dc916e4b22d8547b7e27f8ecf238/R/shiny.R#L512
  • Change misc. layer = layer to layer = layer_id
    • https://github.com/mhpob/mapgl/blob/e96a53166d86dc916e4b22d8547b7e27f8ecf238/R/shiny.R#L180
    • https://github.com/mhpob/mapgl/blob/e96a53166d86dc916e4b22d8547b7e27f8ecf238/R/shiny.R#L197
    • https://github.com/mhpob/mapgl/blob/e96a53166d86dc916e4b22d8547b7e27f8ecf238/R/shiny.R#L207
    • https://github.com/mhpob/mapgl/blob/e96a53166d86dc916e4b22d8547b7e27f8ecf238/R/shiny.R#L227
    • https://github.com/mhpob/mapgl/blob/e96a53166d86dc916e4b22d8547b7e27f8ecf238/R/shiny.R#L262
    • https://github.com/mhpob/mapgl/blob/e96a53166d86dc916e4b22d8547b7e27f8ecf238/R/shiny.R#L272
    • https://github.com/mhpob/mapgl/blob/e96a53166d86dc916e4b22d8547b7e27f8ecf238/R/shiny.R#L471
    • https://github.com/mhpob/mapgl/blob/e96a53166d86dc916e4b22d8547b7e27f8ecf238/R/shiny.R#L487
    • https://github.com/mhpob/mapgl/blob/e96a53166d86dc916e4b22d8547b7e27f8ecf238/R/shiny.R#L535
    • https://github.com/mhpob/mapgl/blob/e96a53166d86dc916e4b22d8547b7e27f8ecf238/R/shiny.R#L551

mhpob avatar Mar 28 '25 16:03 mhpob

@walkerke I'm opening this PR in draft as this line also contains a reference to "layer = layer". I'm unsure if this is referring to underlying JS (don't change) or the object (should probably change). If you could give me direction on this, I'll tie it up and submit the PR.

mhpob avatar Mar 28 '25 16:03 mhpob

@mhpob in that example, we are referring to the layer specifically as we add it to the map - so we won't want to change that / can leave that function untouched.

walkerke avatar Mar 31 '25 15:03 walkerke

We ended up doing some edits that introduced a bunch of merge conflicts, so I ended up handling separately to streamline. I do appreciate the contribution though!

walkerke avatar Jun 15 '25 17:06 walkerke