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

Show clicked Bounding Box in BB-Tab #7935

Merged
merged 7 commits into from
Jul 29, 2024

Conversation

MichaelBuessemeyer
Copy link
Contributor

@MichaelBuessemeyer MichaelBuessemeyer commented Jul 19, 2024

URL of deployed dev instance (used for testing):

Steps to test:

  • Create an annotation of your favourite dataset.
  • Use the bbox tool to create a lot of bounding boxes.
  • Whenever a new bounding box is created, the bounding box tab should scroll to the bottom and highlight this bounding box.
  • Create a lot of bounding boxes to get into a scrolling state in the bbox tab
  • When a bounding box is clicked while the bbox tool is active, the bbox should also be highlighted.
  • Same goes for resizing

TODOs:

Open Questions

  • Should the context menu support to highlight a bounding box? -> Not necessary now.

Issues:


(Please delete unneeded items, merge only when none are left open)

@MichaelBuessemeyer MichaelBuessemeyer marked this pull request as ready for review July 22, 2024 12:22
Copy link
Member

@daniel-wer daniel-wer left a comment

Choose a reason for hiding this comment

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

Nicely done, works very well 👍

One more thing that came to my mind, because the view is not the fastest having 20+ bounding boxes (also hasn't been before!): Now that an antd table is used one could try to set the virtual prop and check whether that improves performance. There's no need to do that in this PR, but if you'd like you can try.

@MichaelBuessemeyer
Copy link
Contributor Author

Sorry @daniel-wer, but I did some code adjustments that need your review:

  • I moved the +-button into the footer of the table.
  • No longer rendering an empty table in view mode; Instead, nothing is rendered.
  • The id prop was removed from the bounding box inputs as a ref to the table supports scrolling the table to a specific element. I did not find this earlier in the docs (only found it while testing virtualizing the table). => The new highlighting does not have a scrolling animation anymore. Afaik this also goes for the other tabs, so I thought this would be fine.

I also tested making the table virtualized but stopped doing so, as it sadly requires setting a specified scroll width and height. In order to get the correct values of width and height I would have needed something like hardcoded calculation of the tab's current width and height. This is due to the tab being resizable. As I wanted to avoid this rather ugly code, I undid my changes and kept the table in a not virtualized state.

So this remains a future TODO.

Could you please recheck the newest version of this PR @daniel-wer

@MichaelBuessemeyer
Copy link
Contributor Author

And @daniel-wer what do you think about this:

Should the context menu support to highlight a bounding box?

Copy link
Member

@daniel-wer daniel-wer left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Thank you for testing the virtualization! I second your opinion that it's best to look at that as a followup.

Should the context menu support to highlight a bounding box?

I don't think that is necessary as simply clicking on the bounding box will highlight it in the list.

@MichaelBuessemeyer MichaelBuessemeyer merged commit 78a4974 into master Jul 29, 2024
2 checks passed
@MichaelBuessemeyer MichaelBuessemeyer deleted the allow-having-selected-bounding-box branch July 29, 2024 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Select User Bounding Boxes from Viewports
2 participants