-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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(mobile): Folder View for mobile #15047
base: main
Are you sure you want to change the base?
Conversation
import 'package:immich_mobile/entities/asset.entity.dart'; | ||
|
||
abstract interface class IFolderApiRepository { | ||
Future<AsyncValue<List<String>>> getAllUniquePaths(); |
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.
We should just return the Future<List<String>>
here. So that we don't add additional type dependency from riverpod
library to the repository
final String path; | ||
List<Asset>? assets; | ||
final List<RecursiveFolder> subfolders; | ||
final FolderService _folderService; |
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.
I find it strange to include the FolderService
here. I think the Model should serve solely as a way to represent the data structure and not include dependencies in its structure
page: FolderRoute.page, | ||
guards: [_authGuard], | ||
transitionsBuilder: TransitionsBuilders.slideLeft, | ||
), |
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.
I find using TransitionsBuilders.fadeIn
is nicer here
fetchFolders(); | ||
} | ||
|
||
Future<void> fetchFolders() async { |
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.
You can use the AsyncValue
's methods in this function to represent the loading/error states
Thanks for the feedback Alex, I implemented your suggested fixes as well as I could. I have now tried to implement the loading of assets by creating a new Provider, however something is not working yet. |
General Idea
Implementation Idea
My rough Idea was to have one single state representing the entire folder structure, since that can be derived from a single api call.
And then, whenever a folder gets navigated to, the assets for that specific folder could get fetched, and also saved in that state.
That way (I hope) you could navigate out and back in again without having to refetch the assets, since they would still be saved in state.
So far
Since I have basically no experience with complex Flutter projects or Flutter State Management, this is gonna be rough, but i've hacked together a somewhat functional prototype that fetches the folders and lets you navigate through them.
Asset loading is not yet implemented, since I am sure there are already plenty of basic improvements that could be made at this stage from more experience devs regarding the entire state and fetching logic.