-
-
Notifications
You must be signed in to change notification settings - Fork 457
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
✨ Support Multiple Special Items #635
base: main
Are you sure you want to change the base?
Conversation
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 like this idea, would be better if we go further to support dynamic grid positions. 😆
3f0a752
to
776ab18
Compare
bool get shouldBuildSpecialItem => | ||
specialItemPosition != SpecialItemPosition.none && | ||
specialItemBuilder != null; | ||
bool get shouldBuildSpecialItem => specialItems.isNotEmpty; |
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.
The semantics of this field is broken since it no longer respects the builder.
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.
any suggestion for this?
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.
Maybe we can get rid of it.
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.
If shouldBuildSpecialItem
is removed, then it will always show loading indicator even there are special item to be displayed. Below is the current code which depends on shouldBuildSpecialItem
.
final bool shouldDisplayAssets =
p.hasAssetsToDisplay || shouldBuildSpecialItem;
return AnimatedSwitcher(
duration: switchingPathDuration,
child: shouldDisplayAssets
? Stack(
children: <Widget>[
RepaintBoundary(
child: Column(
children: <Widget>[
Expanded(child: assetsGridBuilder(context)),
bottomActionBar(context),
],
),
),
pathEntityListBackdrop(context),
pathEntityListWidget(context),
],
)
: loadingIndicator(context),
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.
shouldBuildSpecialItem
has been removed, this is because assetsGridBuilder
has handled the total asset count checking to display either loadingIndicator or list. Below is the code in assetsGridBuilder
.
final List<SpecialItemModel> specialItemModels = specialItems
.map((item) {
final specialItem = item.builder?.call(
context,
wrapper?.path,
totalCount,
permissionNotifier.value,
);
if (specialItem != null) {
return SpecialItemModel(
position: item.position,
item: specialItem,
);
}
return null;
})
.whereType<SpecialItemModel>()
.toList();
totalCount += specialItemModels.length;
if (totalCount == 0 && specialItemModels.isEmpty) {
return loadingIndicator(context);
}
bool get shouldBuildSpecialItem => | ||
specialItemPosition != SpecialItemPosition.none && | ||
specialItemBuilder != null; | ||
bool get shouldBuildSpecialItem => specialItems.isNotEmpty; |
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.
Maybe we can get rid of it.
I think we are in a relatively good state. Thanks for all your contributions! However, I'm planning some API-breaking changes such as #639, that will be pushed into the next major release, so I want to hold them together. |
Co-authored-by: Alex Li <github@alexv525.com>
Co-authored-by: Alex Li <github@alexv525.com>
Co-authored-by: Alex Li <github@alexv525.com>
Co-authored-by: Alex Li <github@alexv525.com>
Co-authored-by: Alex Li <github@alexv525.com>
3d8f331
to
054752c
Compare
✨ What's the context?
Current
AssetPickerConfig
only accept singlespecialItemPosition
andspecialItemBuilder
which is is not suitable for cases where multiple special items is required.🛠 Changes being made
specialItems
inAssetPickerConfig
and which accept list ofspecialItemPosition
andspecialItemBuilder
.isPermissionLimited
param toSpecialItemBuilder
typedef for case where special item is required to remove whenisPermissionLimited
is false. Example as below.SpecialPosition.none
enum.✨ Result