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

Update UI for identifications and show multiple suggestions per classification #741

Open
wants to merge 27 commits into
base: main
Choose a base branch
from

Conversation

annavik
Copy link
Member

@annavik annavik commented Feb 26, 2025

Summary

In this PR we implement the new design for identifications. This includes showing multiple suggestions per classificatoins.

List of Changes

  • Use new components to present identifications
  • Fetch classification details when a machine prediction is expanded
  • Show multiple suggestions per classification
  • Update bunch of other related components to use our new building blocks from Nova, to make UI more streamlined

Related Issues

Detailed Description

We have going a bit back and forth around these changes. The initial design (see Figma) suggested separating "confirming identifications" from "custom identifications" and also a grouped structure. We did like these ideas, however they would mean some significant logic changes, which we decided we are not fully ready for. We have a few stakeholders that needs to be involved in such decisions, some active labeling projects to consider and also some more high prio tasks. Also, our most urgent need for identifications was to present machine predictions in a more detailed way. Therefore, we decided to skip the suggested logic updates on the backend side, focus on our most urgent needs, but still try use as much as possible from the updated design, with some compromises.

How to Test the Changes

Please test all features related to identifications:

  • Confirm machine prediction
  • Confirm human suggestion
  • Suggest ID
  • Expand machine prediction
  • Apply classification from an expanded item
  • Delete identification

Screenshots

Before:

Screenshot 2025-02-26 at 15 15 27

After:

Screenshot 2025-02-26 at 15 12 53 Screenshot 2025-02-26 at 15 12 58

Deployment Notes

This PR needs a small update after the permission PR (see #693) is merged!

Copy link

netlify bot commented Feb 26, 2025

Deploy Preview for antenna-preview ready!

Name Link
🔨 Latest commit 977b4e2
🔍 Latest deploy log https://app.netlify.com/sites/antenna-preview/deploys/67e12ff22fba190008911e92
😎 Deploy Preview https://deploy-preview-741--antenna-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 71 (🔴 down 4 from production)
Accessibility: 89 (no change from production)
Best Practices: 92 (no change from production)
SEO: 100 (no change from production)
PWA: 80 (no change from production)
View the detailed breakdown and full score reports

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

@annavik annavik requested a review from mihow February 26, 2025 14:47
@annavik
Copy link
Member Author

annavik commented Feb 26, 2025

@mihow I did notice that if I try to fetch classification details, I get 500-responses from the backend in some cases. This is how we present such a failure in UI:
Screenshot 2025-02-26 at 15 54 25
Screenshot 2025-02-26 at 15 54 19

On localhost, this is the case for the initial occurrences I get, if I spin up a new database. If I process captures, the occurrences that will be generated can be expanded without any problems.

Is this something to worry about? Maybe it's just the dummy database that needs to be updated, or would this affect all older classifications?

@mihow
Copy link
Collaborator

mihow commented Feb 26, 2025

@mihow I did notice that if I try to fetch classification details, I get 500-responses from the backend in some cases. This is how we present such a failure in UI: Screenshot 2025-02-26 at 15 54 25 Screenshot 2025-02-26 at 15 54 19

On localhost, this is the case for the initial occurrences I get, if I spin up a new database. If I process captures, the occurrences that will be generated can be expanded without any problems.

Is this something to worry about? Maybe it's just the dummy database that needs to be updated, or would this affect all older classifications?

Thanks, yes it shouldn't throw a 500 error if there are not top N choices. I will look into that.

@mihow
Copy link
Collaborator

mihow commented Mar 7, 2025

I found some cases where there is top-n data but the species values are null (even though there are scores). Mainly for the moth/non-moth model. So I need a little more time on this one.

@mihow mihow added this to the ML features v2 milestone Mar 11, 2025
@mihow
Copy link
Collaborator

mihow commented Mar 11, 2025

@annavik Will you fix the merge conflicts while we wait for me to fix the backend issue?

@annavik annavik force-pushed the feat/update-identifications branch from d2e5404 to 8c54374 Compare March 13, 2025 14:58
@annavik
Copy link
Member Author

annavik commented Mar 13, 2025

@annavik Will you fix the merge conflicts while we wait for me to fix the backend issue?

Done! I have synced this with main and included some final tweaks as well. I think this one is ready now!

This is how it looks if we get some error:
Screenshot 2025-03-13 at 17 08 37

This is how it looks if we get a response, but we have nothing to show:
Screenshot 2025-03-13 at 17 11 02

@mihow
Copy link
Collaborator

mihow commented Mar 14, 2025

@mihow todo: only return the full list of logits & scores when requests explicitly by the user (query param or another view)

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.

Show more than the first prediction in UI
2 participants