-
Notifications
You must be signed in to change notification settings - Fork 1
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
base: master
Are you sure you want to change the base?
Update annotation import #2214
Conversation
Size Change: +7.83 kB (+0.2%) Total Size: 3.92 MB
ℹ️ View Unchanged
|
src/app/components/import-annotations/add-annotations/add-annotations.component.html
Outdated
Show resolved
Hide resolved
src/app/components/import-annotations/add-annotations/add-annotations.component.ts
Outdated
Show resolved
Hide resolved
</p> | ||
|
||
<div> | ||
<ngx-datatable |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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
src/app/components/import-annotations/add-annotations/add-annotations.component.ts
Outdated
Show resolved
Hide resolved
src/app/components/import-annotations/add-annotations/add-annotations.component.ts
Outdated
Show resolved
Hide resolved
src/app/services/baw-api/audio-event-import-file/audio-event-import-file.service.ts
Show resolved
Hide resolved
import annotations button should be flushed right |
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) |
Since we support importing multiple files, we should
Something like this:
|
src/app/services/baw-api/audio-event-import-file/audio-event-import-file.service.spec.ts
Outdated
Show resolved
Hide resolved
This was previously possible, but did not include a bunch of edge cases. E.g. uploading a duplicate file
This was a bug from using iif in the previous commit
162e09b
to
175a87a
Compare
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
AnnotationImportFile
models & servicesbawVirtualDatatablePagination
directive so that we can use table pagination without an API backingtypeaheadCallback
method to thetags.service
AnnotationImport
modelImportedAudioEvent
modelAbstractModel
getJsonAttributes(opts: Xor<{ create: boolean} | { update: boolean }>)
has been updated togetJsonAttributesForCreate
andgetJsonAttributesForUpdate
baw-api
servicecreate
andupdate
methods now accept aparams
object for unscoped form dataBawApiError
s to provide the responsesdata
AnnotationImportFileRead
andAnnotationImportFileWrite
models (models returned for reading / writing used to be different. This was fixed in the API refactor)Issues
Fixes: #2200
Potential future changes
Visual Changes
Annotation import details page (events)
Annotation import details page (files)
Add new annotations page (with no uploaded files)
Add new annotations page (with errors)
(note: with the new columns, the table is now horizontally scrollable)
Final Checklist
npm run lint
)npm run test:all
)