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

Add image and video support #79

Merged
merged 9 commits into from
Jul 31, 2024
Merged

Conversation

gjmooney
Copy link
Collaborator

@gjmooney gjmooney commented Jul 29, 2024

Working towards #11. Adds image and video layers and sources.

video_layer

image_layer

Copy link
Contributor

Binder 👈 Launch a Binder on branch gjmooney/jupytergis/add_video_support

@gjmooney gjmooney added the enhancement New feature or request label Jul 29, 2024
@gjmooney gjmooney changed the title Add video support Add image and video support Jul 29, 2024
Copy link
Member

@martinRenou martinRenou left a comment

Choose a reason for hiding this comment

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

Neat!

Couple of comments (we can resolve this in follow-up PRs):

  • it would be great to have a Python API for this too add_video / add_image
  • it would be nice to have a toolbar button for image layer and one for video layer which allows creating both the source and the layer at once
  • I'm seeing this in the form, I'm not sure if it has to do with my recent PR Improve form CSS #74 or if it's related to yours
    Screenshot from 2024-07-31 08-51-26

@martinRenou
Copy link
Member

Concerning the form rendering issue, those big buttons that are messing up the rendering should actually be removed by the logic in https://github.com/QuantStack/jupytergis/blob/main/packages/base/src/formbuilder/objectform/baseform.tsx#L128

For a bit of contexts, those buttons are for editing arrays (removing items, moving items around, adding items). We have this logic to remove those buttons because it does not quite make sense to reorder or add/remove items from the arrays in our case.

@martinRenou
Copy link
Member

Ah I guess it's because your coordinates is an array of arrays and we're not handling that case when removing the array edit buttons.

@gjmooney gjmooney force-pushed the add_video_support branch from 46fd905 to cb18965 Compare July 31, 2024 08:07
@gjmooney
Copy link
Collaborator Author

it would be nice to have a toolbar button for image layer and one for video layer which allows creating both the source and the layer at once

I plan on refactoring our commands a bit (something like this) and wanted to use that as a chance to make sure the toolbar/context menus have all the options we've implemented so far.

Ah I guess it's because your coordinates is an array of arrays and we're not handling that case when removing the array edit buttons.

I was able to apply the ui:options to the sub-arrays so now it looks like

new_coords

Copy link
Member

@martinRenou martinRenou left a comment

Choose a reason for hiding this comment

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

Thanks!

@martinRenou martinRenou merged commit 051dfa0 into geojupyter:main Jul 31, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants