feat: replace map _height with height
#816
Conversation
…string for the height of the lonboard div. set the map div inside the lonboard div to be 100% height/width always
…d by the App function in index.tsx
…not an integer so we can pass values like '300px' or '100%' set the height/width of the layout of the widget to 100% mark the body overflow to hidden so if it's inside a smaller container, it wont overrun it or make some scroll bars show up
…ionality of how `_height` worked
shows the behavior of the height functionality
ATL2001
changed the title
_height with height_height with height
lonboard/_map.py
Outdated
| @@ -177,7 +179,7 @@ def on_click(self, callback: Callable, *, remove: bool = False) -> None: | |||
| Indicates if a click handler has been registered. | |||
| """ | |||
|
|
|||
| _height = t.Int(default_value=DEFAULT_HEIGHT, allow_none=True).tag(sync=True) | |||
| height = t.Unicode(default_value=DEFAULT_HEIGHT).tag(sync=True) | |||
There was a problem hiding this comment.
I don't really want to have a breaking change here, so if we allow strings here, it should be a t.Union of strings and integers (or at least we need to validate that we can pass in an integer and it'll get converted to a string, but we probably want to add a px suffix?)
|
It would be really nice if the HTML export automatically filled the entire window: Map([]).to_html("test.html")gives: |
|
oh yeah, this exporting as height 100% looks really sharp! you want a map, you get a map! 💥 |
lonboard/_map.py
Outdated
| ) | ||
| original_height = self.height | ||
| try: | ||
| self.height = "100%" |
There was a problem hiding this comment.
Will this cause the JS side to be janky? Is there a way to set this without causing a JS side re-render?
Like maybe use hold_trait_notifications to avoid sending any new events while temporarily setting .height? https://traitlets.readthedocs.io/en/stable/using_traitlets.html#holding-trait-cross-validation-and-notifications
There was a problem hiding this comment.
good call, I didn't notice that re-rendering happening
src/index.tsx
Outdated
| let height = String(mapHeight); | ||
| if (typeof mapHeight === "number") { | ||
| // if a number is used for height, assume pixels | ||
| height = height + "px"; | ||
| } |
There was a problem hiding this comment.
I was thinking of suggesting this validation be done on the Python side, so the Python side is always sending a string to the JS side. There are several places that we already use custom validators
…ck when going to html. convert height to string on python side
|
To fix CI you can run |
|
I was planning on deleting that notebook eventually, but i went ahead and fixed it up, but I'm not sure what this new error means :( |
|
If you look at the git diff, you did something to change the symlinks. And so the docs website build isn't able to find the |
per what we discussed in #781 I believe I've come up with a good solution here. I had to add some CSS to the data-overlay-container which appears to be generated by the typescript which the lonboard div is housed in. and then I needed to make some tweaks to the map div which lives inside the lonboard one as well. then I changed
_heighttoheightand tweaked the way the default was setup so as to not make the default functionality of the creation of the map change.but now if the user knows they want to put the map into an IFrame or some other container that has a fixed height, they can specify
height="100%"and the map will fill the container's height.I also checked in an example notebook so you can check the behavior yourself. I do not intend to keep that in there (unless you think it would be a benefit for others, if so I should probably retool it to make it a little nicer)
If this looks good let me know and I can find some time to update the docs so they match the behavior 👍