MaikuB
[flutter_local_notifications] Add support to pass bubble activity + intent to notification
Interesting! Perhaps you can make another Dart class for the bubble details? That way they stay bundled together and you can just use ?. to check its members instead of checking each one independently (eg, what happens if bubbleActivity == null && bubbleExtra != null? This way you can just make AndroidBubble.activity be required and AndroidBubble.extra be nullable).
Then be sure to properly handle the error case in your Java code.
And of course, please be sure to document the Dart parts, any extra Android-specific setup, and add a test case if possible, and let us know when you're finished. I'd be happy to test on my end as well.
@Airyzz I received notifications that you've been updating your fork and wanted to ping to see if there were comments left on the PR to see if you look into. If not then I'll unfortunately may have to close this as it's been a while since this has been opened. I'm also conscious of making attempts to update this directly as you hadn't created a separate branch on your fork
hey, I'll work on implementing your feedback soon, i sort of forgot about this PR 😅
I saw you pushed some changes now but I'm not sure if you're done so let know when it's ready for a review. I spotted some things I could comment on already but thought better to hold off until I know you're done
For future reference, if you intend to submit a PR to another repo like this one, one thing I was taught was to create branch on the fork and use that branch to hold the changes that you intend to contribute. This way the default branch (e.g. main/master) on your fork is solely reserved for keeping it up to date with the original repo you forked from. It makes it easier to maintain a fork as well.
Sorry, it's not ready yet, i'll convert this to a draft for now until its ready
No worries. A general note I can mention is that I've put some information in the contribution guide. One of the main things, I wanted to shout out on is around API docs that can be applied to the PR
No problem, i'll go in and document everything once I'm finished with the rest
Apart from your previous comment about handling exceptions, I think this is ready to look at
Ok I'll use the PR comments to explain further around that. Note I've not gotten to test the changes in the PR yet
Great work with the PR. It will be great to have it. I guess it addresses #209 , though old , I still got there from searching.
Hey @Airyzz, I saw you had recently updated your fork so wanted to check to see if you'd be able to look at the comments that were left on the PR