Skip to content

add: index param in selectAsset #399

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

Merged
merged 4 commits into from
Dec 26, 2022

Conversation

LeGoffMael
Copy link
Collaborator

This PR expose grid index param in the selectAsset delegate function.

@AlexV525
Copy link
Member

AlexV525 commented Dec 23, 2022

What is the purpose for the index? This will be a breaking change.

@LeGoffMael
Copy link
Collaborator Author

LeGoffMael commented Dec 23, 2022

For my case, the goal is to get the position of the selected asset in the grid view, so i can move the scroll controller to it (same behavior as instagram).

final row = (index / gridCount).floor();
final size =
    (MediaQuery.of(context).size.width - itemSpacing * (gridCount - 1)) /
        gridCount;
gridScrollController.jumpTo(row * size + (row * itemSpacing));

here is a record of the scroll animation that requires the index:

RPReplay_Final1671784300.MP4

@AlexV525
Copy link
Member

Please add a changelog, a migration guide, and bump the version to 8.3.0 in both the package and the example (the example requires version code addition too).

@LeGoffMael
Copy link
Collaborator Author

done~

@AlexV525
Copy link
Member

BTW, the picker looks beautiful in the recording. It would be great if you're willing to contribute it as an example in the repo.

@LeGoffMael
Copy link
Collaborator Author

LeGoffMael commented Dec 26, 2022

I applied your remarks in the migration guide and the changelog.

BTW, the picker looks beautiful in the recording. It would be great if you're willing to contribute it as an example in the repo.

Thank you~! It actually try to mimic the instagram picker, because i also needed the crop view and a controller, i made a repo (still in development): insta_assets_picker

I did not submit it as a custom delegate because the crop feature seems too out of scope. But if you're interested i could replace the crop view by a simple carousel and submit InstaAssetPickerBuilder.

@AlexV525
Copy link
Member

But if you're interested i could replace the crop view by a simple carousel and submit InstaAssetPickerBuilder.

That would be nice!

Copy link
Member

@AlexV525 AlexV525 left a comment

Choose a reason for hiding this comment

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

LGTM

@AlexV525 AlexV525 merged commit af12849 into fluttercandies:main Dec 26, 2022
@AlexV525
Copy link
Member

@all-contributors Add @LeGoffMael for code.

@allcontributors
Copy link
Contributor

@AlexV525

I've put up a pull request to add @LeGoffMael! 🎉

AlexV525 pushed a commit that referenced this pull request Dec 26, 2022
Adds @LeGoffMael as a contributor for code.

This was requested by AlexV525 [in this
comment](#399 (comment))

[skip ci]

Co-authored-by: allcontributors[bot] <46447321+allcontributors[bot]@users.noreply.github.com>
AlexV525 added a commit that referenced this pull request Dec 30, 2022
As we discussed previously #399.
Introduce `InstaAssetPickerBuilder` based on
[insta_assets_picker](https://github.com/LeGoffMael/insta_assets_picker)
which reproduces the Instagram layout.

### This custom delegate shows how to:
- Setup a completely different: layout, select and view logic.
- Use `keepScrollOffset` when set to true to restore scroll and preview.
- Customize `DefaultAssetPickerViewerBuilderDelegate`
  (there is no example for it at present).

It was tested on both iOS and Android.

### Changes needed outside of the example folder
- Fixed an issue with `AssetPickerViewer` when the viewer is being used in nested widgets,
  `initStateAndTicker` would be omitted.

Co-authored-by: Alex Li <[email protected]>
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.

2 participants