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 annotation import #2214

Open
wants to merge 93 commits into
base: master
Choose a base branch
from
Open

Conversation

hudson-newey
Copy link
Member

@hudson-newey hudson-newey commented Feb 12, 2025

Update annotation import

Following breaking changes to the annotation import API, I have restructured the annotation import API and UI to reflect these changes.

Changes

  • Adds AnnotationImportFile models & services
  • Adds error card component
  • Adds bawVirtualDatatablePagination directive so that we can use table pagination without an API backing
  • Adds new typeaheadCallback method to the tags.service
  • Adds handling of edge case where the typeahead input doesn't have a callback instantiated
  • Updates AnnotationImport model
  • Updates ImportedAudioEvent model
  • AbstractModel getJsonAttributes(opts: Xor<{ create: boolean} | { update: boolean }>) has been updated to getJsonAttributesForCreate and getJsonAttributesForUpdate
  • The baw-api service create and update methods now accept a params object for unscoped form data
  • Updates BawApiErrors to provide the responses data
  • Removes AnnotationImportFileRead and AnnotationImportFileWrite models (models returned for reading / writing used to be different. This was fixed in the API refactor)

Issues

Fixes: #2200

Potential future changes

  • When clicking on the "Add annotations" button, it should take you to a combined page to create a new annotation import and add files
  • Users might want to edit their annotation before they commit them. (have input boxes so that they can change the values)

Visual Changes

image

Annotation import details page (events)

image

Annotation import details page (files)

image

Add new annotations page (with no uploaded files)

image

Add new annotations page (with errors)

(note: with the new columns, the table is now horizontally scrollable)

Final Checklist

  • Assign reviewers if you have permission
  • Assign labels if you have permission
  • Link issues related to PR
  • Ensure project linter is not producing any warnings (npm run lint)
  • Ensure build is passing on all browsers (npm run test:all)
  • Ensure CI build is passing
  • Ensure docker container is passing (docs)

@hudson-newey hudson-newey added the bug Something isn't working label Feb 12, 2025
@hudson-newey hudson-newey self-assigned this Feb 12, 2025
Copy link
Contributor

github-actions bot commented Feb 12, 2025

Size Change: +7.83 kB (+0.2%)

Total Size: 3.92 MB

Filename Size Change
dist/workbench-client/browser/index.html 4.81 kB +1 B (+0.02%)
dist/workbench-client/browser/main-DFLLPEEW.js 0 B -1.13 MB (removed) 🏆
dist/workbench-client/server/main.js 1.95 MB +4.36 kB (+0.22%)
dist/workbench-client/browser/main-PBOGZX2Q.js 1.14 MB +1.14 MB (new file) 🆕
ℹ️ View Unchanged
Filename Size
dist/workbench-client/browser/assets/buffer-builder-processor-BhnxGUx8.js 1.16 kB
dist/workbench-client/browser/assets/environment.json 555 B
dist/workbench-client/browser/assets/high-accuracy-time-processor-BevUJNwo.js 354 B
dist/workbench-client/browser/assets/test-assets/index.html 21 B
dist/workbench-client/browser/chunk-NPDQGTTA.js 1.17 kB
dist/workbench-client/browser/chunk-U5EWAFH4.js 379 kB
dist/workbench-client/browser/manifest.json 147 B
dist/workbench-client/browser/polyfills-C5CKP5CH.js 12.4 kB
dist/workbench-client/browser/styles-ETV6J7SM.css 39.6 kB
dist/workbench-client/server/267.js 390 kB
dist/workbench-client/server/327.js 4.21 kB

compressed-size-action

@hudson-newey hudson-newey marked this pull request as ready for review February 13, 2025 00:06
@hudson-newey hudson-newey changed the title [DRAFT] Update annotation import Update annotation import Feb 13, 2025
Copy link
Contributor

github-actions bot commented Feb 14, 2025

Unit Test Results

         6 files           6 suites   11m 11s ⏱️
24 564 tests 23 808 ✔️ 756 💤 0
24 804 runs  24 048 ✔️ 756 💤 0

Results for commit 175a87a.

♻️ This comment has been updated with latest results.

</p>

<div>
<ngx-datatable
Copy link
Member

Choose a reason for hiding this comment

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

can this data table show events from more than one file? If so, we need a file column.

Probably need an index column too.

Maybe this could be combined:

ID (tooltip: "file index and even index in each file")
0:0
1:0
1:1234

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes it can, I like the idea

@atruskie
Copy link
Member

Also just realized, these are the fields that are parsed:

image

So they should all appear in the grid

@atruskie
Copy link
Member

import annotations button should be flushed right

@atruskie
Copy link
Member

Shorten "no associated tags" (empty tags template) to just "none" or "empty". Use a style to indicate it's a special value (maybe italics, and grey color)

@atruskie
Copy link
Member

Since we support importing multiple files, we should

  1. list out each file being imported
  2. make it possible to add additional tags to each individual file.

Something like this:

<index>. <remove button> <name> <error> <add tags> <additional_tags_to_be_added>

@hudson-newey hudson-newey force-pushed the update-annotation-import branch from 162e09b to 175a87a Compare February 21, 2025 02:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade annotation import api service to new baw-server version format
2 participants