Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[AND-207] Simplify notification overriding for customization #1265

Merged
merged 5 commits into from
Jan 6, 2025
Merged

[AND-207] Simplify notification overriding for customization #1265

merged 5 commits into from
Jan 6, 2025
+208 −86

Conversation

rahul-lohra
Copy link
Contributor

@rahul-lohra rahul-lohra commented Jan 2, 2025 •
edited
Loading

🎯 Goal

We received an issue report here: #1214
Notion: https://www.notion.so/stream-wiki/Notifications-Customisation-16f6a5d7f9f680ccbd2fe3bf8686bba1

The report highlights that customizing notifications in our SDK is quite challenging. By customization, it refers to:

  1. Accessing existing pending intents so that they can be integrated into user-defined buttons.
  2. Modifying the notification style.

🛠 Implementation details

We are making these methods from class named DefaultStreamIntentResolver as public

Method Name
searchIncomingCallPendingIntent
searchNotificationCallPendingIntent
searchMissedCallPendingIntent
getDefaultPendingIntent
searchLiveCallPendingIntent
searchAcceptCallPendingIntent
searchRejectCallPendingIntent
searchEndCallPendingIntent

🎨 UI Changes

Add relevant screenshots

Before After
img img

Add relevant videos

Before After

🧪 Testing

Explain how this change can be tested (or why it can't be tested)

Provide a patch below if it is necessary for testing

Provide the patch summary here 
Provide the patch code here

☑️Contributor Checklist

General

  • I have signed the Stream CLA (required)
  • Assigned a person / code owner group (required)
  • Thread with the PR link started in a respective Slack channel (required internally)
  • PR targets the develop branch
  • PR is linked to the GitHub issue it resolves

Code & documentation

  • Changelog is updated with client-facing changes
  • New code is covered by unit tests
  • Comparison screenshots added for visual changes
  • Affected documentation updated (KDocs, docusaurus, tutorial)
  • Tutorial starter kit updated
  • Examples/guides starter kits updated (stream-video-examples)

☑️Reviewer Checklist

  • XML sample runs & works
  • Compose sample runs & works
  • Tutorial starter kit
  • Example starter kits work
  • UI Changes correct (before & after images)
  • Bugs validated (bugfixes)
  • New feature tested and works
  • Release notes and docs clearly describe changes
  • All code we touched has new or updated KDocs

🎉 GIF

Please provide a suitable gif that describes your work on this pull request

@rahul-lohra
1. Expose the PendingIntent getters for different types of calls as p…
76faf4e 
…ublic APIs.

2. Mark intentResolver with public visibility
@rahul-lohra rahul-lohra force-pushed the feature/rahullohra/notification_customise_improvement branch from 05318fd to 76faf4e Compare January 2, 2025 11:09
@rahul-lohra rahul-lohra marked this pull request as ready for review January 2, 2025 11:09
@rahul-lohra rahul-lohra requested a review from a team as a code owner January 2, 2025 11:09
@rahul-lohra rahul-lohra self-assigned this Jan 2, 2025
@aleksandar-apostolov aleksandar-apostolov linked an issue Jan 3, 2025 that may be closed by this pull request
Can you simplify overriding DefaultNotificationHandler to let customers create own Notification style and look #1214
Closed
aleksandar-apostolov
aleksandar-apostolov reviewed Jan 3, 2025
View reviewed changes
Copy link
Collaborator

@aleksandar-apostolov aleksandar-apostolov left a comment

Choose a reason for hiding this comment

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

We should probably open some of these methods in order to increase further the extensibility of the class. WDYT?

@rahul-lohra
Merge branch 'develop' into feature/rahullohra/notification_customise…
38603c6 
…_improvement
@rahul-lohra
change method visibility to open for enhanced notification customization
3afe63a 
Updated visibility of private and default methods to open to allow further extension and customization of notifications.

This change provides greater flexibility for users to override and tailor notification behavior according to their specific needs.
@rahul-lohra
Copy link
Contributor Author

rahul-lohra commented Jan 6, 2025 •
edited
Loading

We should probably open some of these methods in order to increase further the extensibility of the class. WDYT?

I have opened some of the methods from the class stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/DefaultNotificationHandler.kt

This will further allow the users to modify the

  1. Actions: call, reject and leave action
  2. Notification channels
  3. Showing notifications: Live call, missed Call, general
  4. Override Incoming call notification

@aleksandar-apostolov

@aleksandar-apostolov aleksandar-apostolov enabled auto-merge (squash) January 6, 2025 12:49
@aleksandar-apostolov aleksandar-apostolov merged commit 3b46d50 into develop Jan 6, 2025
5 checks passed
@aleksandar-apostolov aleksandar-apostolov deleted the feature/rahullohra/notification_customise_improvement branch January 6, 2025 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aleksandar-apostolov aleksandar-apostolov aleksandar-apostolov approved these changes

Assignees

@rahul-lohra rahul-lohra

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Can you simplify overriding DefaultNotificationHandler to let customers create own Notification style and look
2 participants
@rahul-lohra @aleksandar-apostolov