Skip to content

Add import validation dialog#655

Merged
subdavis merged 5 commits into
mainfrom
desktop/import-options
Mar 23, 2021
Merged

Add import validation dialog#655
subdavis merged 5 commits into
mainfrom
desktop/import-options

Conversation

@subdavis
Copy link
Copy Markdown
Contributor

@subdavis subdavis commented Mar 19, 2021

Screenshot from 2021-03-19 16-37-16

Screenshot from 2021-03-19 16-41-30

@BryonLewis not ready for code review yet, but if you could give some general feedback about how this tracks against what you had in mind, that'd be appreciated. Is this going in the right direction?

fixes #606
fixes #622

Relevant to parts of #417 as well.

@BryonLewis
Copy link
Copy Markdown
Collaborator

Yeah this is definitely going in the right direction. I'm a little afraid of the complexity that might be added with the other import issues/tasks in our backlog but this is great.

Going to apologize for this brain dump of questions/comments:

  • Is the idea to use the same dialog as a platform for [FEATURE] Desktop: add ability to open image files based on glob selector #622 and [FEATURE] Trans-code videos and run-pipelines in one stage (or just same time) instead of two #530?
    • If so we may want place some of the extra details in a 'details' expandable area to reduce a bit of the clutter of "importing transcoded image sequence with a glob pattern that you want to run an immediate pipeline on". I think you see where I was getting afraid of the complexity. I don't know what to do for this instance while still making it simple for others. I have some minor ideas but everything seems to add complexity.
  • 'Continue to ignore warning' might not be descriptive enough. There is some ambiguity if ignore means overwriting or adding an additional dataset (Although it is indicated as a newID). Especially if only one other dataset with that name/source is found.
  • Dataset Name @ ID might be better swapped to Dataset Name @ Imported date if we save the creation date? I don't think anyone besides us cares about unique IDs. Could possibly add an import date and a modified date.
  • Quick peek at the code looks like you give them option to name a dataset even if there aren't duplicates 👍

@subdavis subdavis force-pushed the desktop/import-options branch from 0bec6d6 to 1114e3b Compare March 20, 2021 20:16
@subdavis subdavis requested a review from BryonLewis March 22, 2021 13:27
@subdavis
Copy link
Copy Markdown
Contributor Author

Ready for review. Your comments were good, I made all those changes.

Copy link
Copy Markdown
Collaborator

@BryonLewis BryonLewis left a comment

Choose a reason for hiding this comment

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

I really like this for the import tool. It makes a lot of sense and gives a good point for some of the other tasks.

Just some things involving edge cases and trying to improperly import data.

Comment thread client/platform/desktop/backend/native/common.ts Outdated
Comment thread client/platform/desktop/frontend/components/ImportDialog.vue
Comment thread client/platform/desktop/frontend/components/ImportDialog.vue
Comment thread client/platform/desktop/frontend/components/ImportDialog.vue Outdated
Comment thread client/platform/desktop/sharedUtils.ts
@subdavis subdavis requested a review from BryonLewis March 22, 2021 21:58
@subdavis subdavis merged commit e008244 into main Mar 23, 2021
@subdavis subdavis deleted the desktop/import-options branch March 23, 2021 00:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants