FoundKeyGang/FoundKey
FoundKeyGang
/
FoundKey
7
19
Fork 22
Code Issues 37 Pull Requests 4 Releases Activity

client refactor: use pagination in drive component #203

Manually merged
Johann150 merged 6 commits from refactor-drive into main 2022-10-17 21:00:44 +00:00
Conversation 2 Commits 6 Files Changed 2 +184 -250
Johann150 commented 2022-10-14 20:20:58 +00:00
Owner
Copy Link

This majorly refactors the drive component to use the proper pagination component instead of reimplementing pagination.

The drive component is also refactored to use ref sugar (i.e. $ref).

Something still to fix is the loading of the two pagination components. On the one hand there are two loading spinners and while they are loading the emptyDrive message will be shown. The problem is that there is currently no way to tell if a pagination is still loading. Either exposing a property or firing a loaded event or something like that might be useful but I'm not sure what is the best solution in this case.

This majorly refactors the drive component to use the proper pagination component instead of reimplementing pagination. The drive component is also refactored to use ref sugar (i.e. `$ref`). Something still to fix is the loading of the two pagination components. On the one hand there are **two loading spinners** ~~and while they are loading **the `emptyDrive` message will be shown**. The problem is that there is currently no way to tell if a pagination is still loading. Either exposing a property or firing a `loaded` event or something like that might be useful but I'm not sure what is the best solution in this case.~~
Johann150 added 2 commits 2022-10-14 20:20:59 +00:00
ci/woodpecker/push/lint-foundkey-js Pipeline was successful Details
ci/woodpecker/push/build Pipeline was successful Details
ci/woodpecker/push/lint-client Pipeline was successful Details
ci/woodpecker/push/lint-backend Pipeline was successful Details
ci/woodpecker/push/test Pipeline was successful Details
6c5ddf839d
client refactor: use pagination in drive component 
This majorly refactors the drive component to use the proper pagination
component instead of reimplementing pagination.

The drive component is also refactored to use ref sugar (i.e. $ref).
ci/woodpecker/push/lint-backend Pipeline was successful Details
ci/woodpecker/push/lint-foundkey-js Pipeline was successful Details
ci/woodpecker/push/lint-client Pipeline was successful Details
ci/woodpecker/push/build Pipeline was successful Details
ci/woodpecker/push/test Pipeline was successful Details
ci/woodpecker/pr/lint-foundkey-js Pipeline was successful Details
ci/woodpecker/pr/lint-backend Pipeline was successful Details
ci/woodpecker/pr/build Pipeline was successful Details
ci/woodpecker/pr/lint-client Pipeline failed Details
ci/woodpecker/pr/test Pipeline failed Details
06a00e5767
remove unavailable i18n strings
Johann150 added this to the (deleted) project 2022-10-16 19:36:16 +00:00
Johann150 added 1 commit 2022-10-16 20:54:01 +00:00
ci/woodpecker/push/lint-client Pipeline was successful Details
ci/woodpecker/push/lint-foundkey-js Pipeline was successful Details
ci/woodpecker/push/lint-backend Pipeline was successful Details
ci/woodpecker/push/build Pipeline was successful Details
ci/woodpecker/push/test Pipeline was successful Details
ci/woodpecker/pr/lint-foundkey-js Pipeline was successful Details
ci/woodpecker/pr/lint-backend Pipeline was successful Details
ci/woodpecker/pr/build Pipeline was successful Details
ci/woodpecker/pr/lint-client Pipeline failed Details
ci/woodpecker/pr/test Pipeline failed Details
5bb119497c
fix: don't display empty drive message while loading
Johann150 commented 2022-10-16 20:55:18 +00:00
Author
Owner
Copy Link

I think i can live with two loading spinners, so marking this as no longer work in progress.

I think i can live with two loading spinners, so marking this as no longer work in progress.
Johann150 changed title from WIP: client refactor: use pagination in drive component to client refactor: use pagination in drive component 2022-10-16 20:55:20 +00:00
norm reviewed 2022-10-16 21:07:13 +00:00
packages/client/src/components/drive.vue Outdated
@ -426,0 +448,4 @@
// display new folder hierarchy appropriately
hierarchyFolders = [];
let parent = foldertoMove.parent;
norm commented 2022-10-16 21:07:13 +00:00
Owner
Copy Link

Getting an error here:

Uncaught (in promise) ReferenceError: foldertoMove is not defined
    at drive.vue:451:16
Getting an error here: ``` Uncaught (in promise) ReferenceError: foldertoMove is not defined at drive.vue:451:16 ```
Johann150 marked this conversation as resolved
Johann150 added 1 commit 2022-10-17 16:56:37 +00:00
ci/woodpecker/pr/lint-foundkey-js Pipeline was successful Details
ci/woodpecker/pr/lint-backend Pipeline was successful Details
ci/woodpecker/pr/build Pipeline was successful Details
ci/woodpecker/push/build Pipeline was successful Details
ci/woodpecker/push/lint-client Pipeline was successful Details
ci/woodpecker/push/lint-backend Pipeline was successful Details
ci/woodpecker/push/lint-foundkey-js Pipeline was successful Details
ci/woodpecker/push/test Pipeline was successful Details
ci/woodpecker/pr/lint-client Pipeline failed Details
ci/woodpecker/pr/test Pipeline failed Details
54a6f1c643
fix variable name
norm commented 2022-10-17 17:51:13 +00:00
Owner
Copy Link

Still can't open folders for some reason, though I'm not seeing anything relevant in the browser console.

Still can't open folders for some reason, though I'm not seeing anything relevant in the browser console.
Johann150 added 1 commit 2022-10-17 19:58:17 +00:00
ci/woodpecker/push/build Pipeline was successful Details
ci/woodpecker/push/lint-client Pipeline was successful Details
ci/woodpecker/push/lint-backend Pipeline was successful Details
ci/woodpecker/push/lint-foundkey-js Pipeline was successful Details
ci/woodpecker/push/test Pipeline was successful Details
ci/woodpecker/pr/lint-backend Pipeline was successful Details
ci/woodpecker/pr/lint-foundkey-js Pipeline was successful Details
ci/woodpecker/pr/build Pipeline was successful Details
ci/woodpecker/pr/lint-client Pipeline failed Details
ci/woodpecker/pr/test Pipeline failed Details
4c82da18b5
fix paginations reloading 
The Pagination type actually specifies that just the params property
should be a Ref.
norm reviewed 2022-10-17 20:22:52 +00:00
packages/client/src/components/drive.vue Outdated
@ -135,0 +162,4 @@
let hierarchyFolders = $ref<foundkey.entities.DriveFolder[]>([]);
let selectedFiles = $ref<foundkey.entities.DriveFile[]>([]);
let selectedFolders = $ref<foundkey.entities.DriveFolder[]>([]);
let keepOriginal = $ref<boolean>(defaultStore.state.keepOriginalUploading); // 外部渡しが多いので$refは使わないほうがよい
norm commented 2022-10-17 20:22:52 +00:00
Owner
Copy Link

Nitpick: Should probably remove or translate the comment here.

Nitpick: Should probably remove or translate the comment here.
Johann150 commented 2022-10-17 20:52:03 +00:00
Author
Owner
Copy Link

Should not use $ref because it is often passed externally

🥴 hmm let me check something

> Should not use $ref because it is often passed externally 🥴 hmm let me check something
Johann150 commented 2022-10-17 20:53:07 +00:00
Author
Owner
Copy Link

Okay there is 1 place where that needs to be passed as a "proper" ref and that is already covered. Just gonna remove that comment.

Okay there is 1 place where that needs to be passed as a "proper" ref and that is already covered. Just gonna remove that comment.
Johann150 marked this conversation as resolved
norm approved these changes 2022-10-17 20:24:21 +00:00
norm left a comment
Owner
Copy Link

Working for me locally on my dev setup. Should be good to go, nitpick aside.

Working for me locally on my dev setup. Should be good to go, nitpick aside.
Johann150 added 1 commit 2022-10-17 20:53:34 +00:00
ci/woodpecker/push/lint-client Pipeline was successful Details
ci/woodpecker/push/lint-backend Pipeline was successful Details
ci/woodpecker/push/build Pipeline was successful Details
ci/woodpecker/push/lint-foundkey-js Pipeline was successful Details
ci/woodpecker/push/test Pipeline was successful Details
ci/woodpecker/pr/lint-backend Pipeline was successful Details
ci/woodpecker/pr/build Pipeline was successful Details
ci/woodpecker/pr/lint-client Pipeline failed Details
ci/woodpecker/pr/lint-foundkey-js Pipeline was successful Details
ci/woodpecker/pr/test Pipeline failed Details
8a9a855ed9
fix: remove comment
Johann150 manually merged commit f4ee8b321e into main 2022-10-17 21:00:44 +00:00
Johann150 deleted branch refactor-drive 2022-10-17 21:00:50 +00:00
Johann150 added the
upkeep
 label 2022-12-23 10:25:53 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
norm
Labels
Clear labels   
feature

task that proposes new feature(s)

  
fix

task to fix behavioral problems in the FoundKey codebase

  
upkeep

task that should make FoundKey smaller and more maintainable

No Label
feature
fix
upkeep
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: FoundKeyGang/FoundKey#203
No description provided.
Delete Branch "refactor-drive"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?