-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
fixed the local ids selecting issue #13256
base: main
Are you sure you want to change the base?
Conversation
@mertalev Please add someone to review |
@shenlong-tanwen updated the implementation inside the |
This reverts commit 04f2ed5.
@shenlong-tanwen Any update on review ? |
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 could do one of the following:
i) You could use your previous implementation of having the filtering logic in the widget and remove all the filtering logic in the provider as it is redundant and has errors
ii) Fix the logical issue in the provider and remove your filtering logic and toast handling in the widget as the assets are already filtered in the provider
Also fix any issues from the dart analyzer as well
Apologies for not being more clearer with the previous comment. Thanks for working on this.
Thanks @shenlong-tanwen , I have updated the logic as per requirement. And, also removed UI logic from the provider code, as you mentioned and have implemented it from code perpective |
|
||
// Trigger a refresh to update the UI after deletion | ||
await ref.read(assetProvider.notifier).getAllAsset(); |
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.
Why is this needed?
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.
Why is this needed?
@yashrajjain726 Gentle reminder
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.
Kindly fix the dart analysis issues. i.e, there are unused imports in the provider file
@yashrajjain726 The changes are good. Can you respond to the only remaining comment on why the refresh is needed? |
@shenlong-tanwen I have replied to it long time ago, please check. |
Fixed #11428