Skip to content

Conversation

@annavik
Copy link
Member

@annavik annavik commented Jul 8, 2023

Adding support for links to a specific capture in session detail view. I thought selections based on capture here was a nice approach (instead of using detections) because it also fits with how we present selections in UI.

If both occurrence and capture are part of the URL, the capture will be prioritised for the first selected capture (I think BE is not doing the same for the offset, but I think maybe it should?).

Example links:

Also preparing detections in occurrence details to link to a specific capture in session details with that occurrence selected. We don't have this info atm, but I hope for it to work if you just add the capture here @mihow:

Screenshot 2023-07-08 at 15 00 41

@annavik annavik requested a review from mihow July 8, 2023 13:09
@netlify
Copy link

netlify bot commented Jul 8, 2023

Deploy Preview for ami-web ready!

Name Link
🔨 Latest commit f3a33b7
🔍 Latest deploy log https://app.netlify.com/sites/ami-web/deploys/64aa628ad3e3fb00091c5309
😎 Deploy Preview https://deploy-preview-195--ami-web.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@annavik annavik changed the title Selected capture Link to a specific capture in session detail view Jul 8, 2023
@netlify
Copy link

netlify bot commented Jul 8, 2023

Deploy Preview for ami-storybook ready!

Name Link
🔨 Latest commit f3a33b7
🔍 Latest deploy log https://app.netlify.com/sites/ami-storybook/deploys/64aa628a7422120008e49954
😎 Deploy Preview https://deploy-preview-195--ami-storybook.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

return {
id,
captureId:
detection.capture !== undefined ? `${detection.capture}` : undefined,
Copy link
Member Author

Choose a reason for hiding this comment

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

If you want to add capture id to occurrence detections using a different key or something, this is where we need to change how to access it @mihow

@mihow
Copy link
Collaborator

mihow commented Jul 9, 2023

@annavik great call with using capture.id to target frames instead of dectection.id, that's much more direct and reliable! I made the changes you suggested to make that possible:

  • The capture param now takes priority in the backend function that calculates the offset. The priority order is: capture.id, timestamp, detection.id, occurrence.id. Which also matches the order of query efficiency (select by capture is the fastest, select by occurrence takes a couple database joins).
  • The API response for occurrence detail now contains nested data about the capture for each detection
  • The detections look for capture.id in occurrence-details.ts

Now the linking from detection thumb to the session detail is much more satisfying. You can click on the last thumb and it will actually go to the last frame (and still select the right occurrence).

image

One small todo (for later) would be to scroll down to the right detection when you navigate from the capture view to the occurrence detail modal.

Also the back button wasn't working for me at one point, but now it is! So maybe something to look out for.

@mihow mihow merged commit ea7dab0 into main Jul 9, 2023
@mihow mihow deleted the web-ui-selected-capture branch August 14, 2023 23:34
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.

3 participants