Skip to content

File-Transfer-App-PR for changes.#54

Open
Akayeshmantha wants to merge 42 commits intozowe:stagingfrom
Akayeshmantha:file-t-1
Open

File-Transfer-App-PR for changes.#54
Akayeshmantha wants to merge 42 commits intozowe:stagingfrom
Akayeshmantha:file-t-1

Conversation

@Akayeshmantha
Copy link
Copy Markdown

@Akayeshmantha Akayeshmantha commented Jul 1, 2020

This PR contains the below changes.

  1. Download service to download larger files.
  2. Config update service ( Which takes queue length and history length)
  3. Conflict resolution of existing CSS files.
  4. Download queue functionality (high priority, low priority)
  5. Showing the history of cancel/ completed downloads.

Akayeshmantha added 25 commits July 1, 2020 12:15
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
Akayeshmantha added 4 commits August 17, 2020 10:55
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
Copy link
Copy Markdown
Member

@DivergentEuropeans DivergentEuropeans left a comment

Choose a reason for hiding this comment

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

So when I do a fresh install & rebuild I get the following errors:

ERROR in ./src/app/browser-panel/browser-panel.component.ts
Module not found: Error: Can't resolve '@zlux/file-explorer/src/plugin' in 'C:\Users\lastrakou\Desktop\zlux\file-transfer-app\webClient\src\app\browser-panel'
 @ ./src/app/browser-panel/browser-panel.component.ts 27:15-56
 @ ./src/app/app.module.ts
 @ ./src/plugin.ts
 @ multi ./src/plugin.ts

ERROR in C:\Users\lastrakou\Desktop\zlux\file-transfer-app\webClient\src\app\app.module.ts
./src/app/app.module.ts
[tsl] ERROR in C:\Users\lastrakou\Desktop\zlux\file-transfer-app\webClient\src\app\app.module.ts(54,32)
      TS2307: Cannot find module '@zlux/file-explorer/src/plugin' or its corresponding type declarations.

ERROR in C:\Users\lastrakou\Desktop\zlux\file-transfer-app\common\FTATypes.ts
../common/FTATypes.ts
[tsl] ERROR in C:\Users\lastrakou\Desktop\zlux\file-transfer-app\common\FTATypes.ts(217,45)
      TS2345: Argument of type 'string | ArrayBuffer' is not assignable to parameter of type 'ArrayBuffer'.
  Type 'string' is not assignable to type 'ArrayBuffer'.

ERROR in C:\Users\lastrakou\Desktop\zlux\file-transfer-app\webClient\src\app\services\FTAActivity.service.ts
./src/app/services/FTAActivity.service.ts
[tsl] ERROR in C:\Users\lastrakou\Desktop\zlux\file-transfer-app\webClient\src\app\services\FTAActivity.service.ts(82,8)
      TS2339: Property 'catch' does not exist on type 'Observable<Response>'.

ERROR in C:\Users\lastrakou\Desktop\zlux\file-transfer-app\webClient\src\app\browser-panel\browser-panel.component.ts
./src/app/browser-panel/browser-panel.component.ts
[tsl] ERROR in C:\Users\lastrakou\Desktop\zlux\file-transfer-app\webClient\src\app\browser-panel\browser-panel.component.ts(16,60)
      TS2307: Cannot find module '@zlux/file-explorer/src/plugin' or its corresponding type declarations.

I tried removing package-lock & node_modules as well, and starting clean but no dice

Also if possible, could you go through and add some comments to a few of the confusing logic bits? Like, you have a lot of sync and async code working side by side, and a service, which is all very good but it's quite hard to read 🙂

Comment thread webClient/src/app/activity-panel/activity-panel.component.ts Outdated
Comment thread webClient/src/app/activity-panel/activity-panel.component.scss Outdated
Comment thread webClient/src/app/activity-table/activity-table.component.scss Outdated
Comment thread webClient/src/app/services/Download.service.ts Outdated
Comment thread webClient/src/app/services/Download.service.ts Outdated
Comment thread webClient/src/app/services/FTAActivity.service.ts
Comment thread webClient/src/app/services/FTAConfig.service.ts
Comment thread webClient/src/environments/environment.prod.ts
Akayeshmantha added 2 commits August 19, 2020 12:18
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
@Akayeshmantha
Copy link
Copy Markdown
Author

@DivergentEuropeans the build is passing now. Also added more comments on the components.

Akayeshmantha added 3 commits August 19, 2020 14:22
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
@Akayeshmantha
Copy link
Copy Markdown
Author

@DivergentEuropeans Lenny all the changes are in place.

Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
@1000TurquoisePogs
Copy link
Copy Markdown
Member

Did others have a problem building this code? I got an out of memory error which was solved by increasing the limit by doing
NODE_OPTIONS="--max-old-space-size=4096"

If you also needed to do this, please put it into the readme or build steps.

@1000TurquoisePogs
Copy link
Copy Markdown
Member

This app really needs UI cleanup. So much work has been done to improve the features & server behavior, but if a user doesn't know what to do or doesn't like how it looks then it won't be used much.
Here are 8 problems I found that should be improved quickly & easily:

  1. The browsing UI takes up a fixed percentage of the window size. It should use 100% of the width, because there is no reason to have empty space on half of the UI.

  2. The queue at the bottom is also invisible initially, despite having tabs. It makes you think the tabs do something but they don't until a download occurs. Having an empty table is better than an invisible one, this will allow the user to understand the purpose of the buttons. You could also make the buttons 'disabled' until there is a queue, so that clicking them does nothing.

  3. The panel on the right is just not properly implemented yet, it needs more time. You should keep the code, but remove the HTML so that people do not see it until it is ready for use. Just remove it from the UI.

  4. Download and upload are confusingly placed. I would have expected to see them next to each other, either left & right or up & down, but download is away from all the other buttons despite having just as important a role as upload. They should be next to each other for this reason.

  5. Cancel is clickable when there is no action happening. You should not be able to click cancel if there is nothing to cancel. If you can cancel in the queue, there is probably no need to have an individual cancel button anyway.

  6. config, cancel, and upload all share the same icon. https://fontawesome.com/v4.7.0/icon/cog is an icon that may be good for "config", aka settings.

  7. I'm not sure what transfer does... is it for the panel in 3 which we should remove from the UI for now? If so, also remove the transfer button. If not, please explain. It's confusing to me.

  8. Primary domain and local:// both have meaning when using the panel in 3, but not right now. Let's remove those too.

@1000TurquoisePogs
Copy link
Copy Markdown
Member

fta

@1000TurquoisePogs
Copy link
Copy Markdown
Member

errors
I'm having trouble downloading, I get many many errors.

@1000TurquoisePogs
Copy link
Copy Markdown
Member

Likewise upload fails: http 500, "could not tag file"
server-side
Could not tag file. Ret: 121, Res: 99091689
ZWES1200W Could not close file: Ret='121', res='0'
The write to disk succeeds, but then the app tries to do tagging upon completion, and that fails.

@1000TurquoisePogs
Copy link
Copy Markdown
Member

autoconv
The toggle button for download doesnt make sense to me... in 'autoconvert' mode, you must select which conversion to do during download. That is not auto, that is manual. Having a manual mode is good, but both auto and manual should exist.

@DivergentEuropeans
Copy link
Copy Markdown
Member

DivergentEuropeans commented Sep 8, 2020

My requested changes I've messaged you privately but yes what @1000TurquoisePogs said, basically UI needs some work and some functions are broken, so maybe let's focus on one or other, instead of attempting to complete this bulk PR perfectly?

@DivergentEuropeans
Copy link
Copy Markdown
Member

Did others have a problem building this code? I got an out of memory error which was solved by increasing the limit by doing
NODE_OPTIONS="--max-old-space-size=4096"

Have this too

@Akayeshmantha
Copy link
Copy Markdown
Author

Did others have a problem building this code? I got an out of memory error which was solved by increasing the limit by doing
NODE_OPTIONS="--max-old-space-size=4096"

If you also needed to do this, please put it into the readme or build steps.

Sure will add that do the build step @1000TurquoisePogs

@Akayeshmantha
Copy link
Copy Markdown
Author

@1000TurquoisePogs @DivergentEuropeans I am working on fixing the issues.

@Akayeshmantha Akayeshmantha force-pushed the file-t-1 branch 2 times, most recently from 3f2303d to e7f353e Compare September 14, 2020 22:35
Signed-off-by: Akayeshmantha <akayeshmantha@apache.org>
Signed-off-by: ayeshmantha.perera <ayeshmantha.perera@salzburgresearch.at>
Signed-off-by: ayeshmantha.perera <ayeshmantha.perera@salzburgresearch.at>
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.

3 participants