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

feat: improve references upload flow #179

Merged
merged 17 commits into from
Jun 22, 2023

Conversation

cguedes
Copy link
Collaborator

@cguedes cguedes commented Jun 21, 2023

This PR adds a new way to upload PDF files into RefStudio.

Now files are added using a drag-and-drop action to anywhere in the app, of using a explicit file browsing action in the references library panel. When a drag operation is ongoing a screen is shown with an indication (for debug) of the files being uploaded.

Features

references.PDF.upload.mov

NOTE: This PR is still missing additional unit tests for the features implemented.

@cguedes cguedes linked an issue Jun 21, 2023 that may be closed by this pull request
@cguedes cguedes mentioned this pull request Jun 21, 2023
11 tasks
@codecov
Copy link

codecov bot commented Jun 21, 2023

Codecov Report

Merging #179 (4f7d575) into main (37d2718) will increase coverage by 20.54%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             main     #179       +/-   ##
===========================================
+ Coverage   65.19%   85.74%   +20.54%     
===========================================
  Files          84       11       -73     
  Lines        4132      435     -3697     
  Branches      305        0      -305     
===========================================
- Hits         2694      373     -2321     
+ Misses       1417       62     -1355     
+ Partials       21        0       -21     

see 72 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@cguedes cguedes requested a review from sehyod June 21, 2023 09:41
@cguedes cguedes linked an issue Jun 21, 2023 that may be closed by this pull request
@cguedes cguedes self-assigned this Jun 21, 2023
src/components/Spinner/index.tsx Outdated Show resolved Hide resolved
src/panels/references/ReferencesDropZone.tsx Outdated Show resolved Hide resolved
src/panels/references/ReferencesDropZone.tsx Outdated Show resolved Hide resolved
src/panels/references/ReferencesDropZone.tsx Outdated Show resolved Hide resolved
@hammer
Copy link
Contributor

hammer commented Jun 22, 2023

Nice! This PR definitely improves #118. Eventually we might want to do some kind of file type detection as I am able to drag any kind of file onto the app and get the upload behavior which is probably not the best user experience.

This PR does not address the first part of #173:

as the library grows, the upload target will move down the screen. Should we consider moving to a tabbed layout with "upload" as a tab next to "library"? Or some other solution?

In this PR, the text TIP: Click here or drag/drop PDF files here for upload. appears below the list of references and hence falls off the screen when there are many references. Perhaps when we do the references table view in #159 we can revisit how the system file browser is accessed and make it always visible?

For the ingestion status bar, if it's hard to communicate percent complete, I'm fine with just show that we're busy someplace. I guess we're going to have to figure out how to handle user actions that are taken while we are busing with the ingestion process. We certainly don't want to block the whole app, but perhaps we want to block new uploads until the previous uploads finish?

@cguedes
Copy link
Collaborator Author

cguedes commented Jun 22, 2023

Nice! This PR definitely improves #118. Eventually we might want to do some kind of file type detection as I am able to drag any kind of file onto the app and get the upload behavior which is probably not the best user experience.

Sure. That's fixed.

In this PR, the text TIP: Click here or drag/drop PDF files here for upload. appears below the list of references and hence falls off the screen when there are many references.

My idea, for the MVP, was that they would see this information when the references list is empty. So they will learn how to upload (click or drag-drop). I agree that we should add a menu entry for References -> Upload and also some other indicator (icon) in the panel for the upload action.

For the ingestion status bar, if it's hard to communicate percent complete, I'm fine with just show that we're busy someplace.

Yes, the idea was to show that we are busy. The %age isn't even working as we don't have that information (we will have after #178).

I guess we're going to have to figure out how to handle user actions that are taken while we are busing with the ingestion process. We certainly don't want to block the whole app, but perhaps we want to block new uploads until the previous uploads finish?

Sure. I decided that for now we won't block them from upload/ingest concurrently but def improve this after that backend part is improved. Also, for the "busy indicator" maybe we should build a fixed bottom bar like VS Code has for loading indicators.

image

@cguedes cguedes requested a review from sehyod June 22, 2023 12:57
@cguedes cguedes marked this pull request as ready for review June 22, 2023 12:59
@cguedes cguedes merged commit 354a762 into main Jun 22, 2023
11 checks passed
@cguedes cguedes deleted the 173-improve-upload-target-for-references branch June 22, 2023 13:25
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.

Improve upload target for references Drag and drop PDF takes over editor with no way out
3 participants