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.

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

SDA-4220 (Implement API for call notification) #1891

Merged
merged 5 commits into from
Jul 16, 2023

Conversation

KiranNiranjan
Copy link
Member

@KiranNiranjan KiranNiranjan commented Jul 11, 2023

Description

Implement call notification API

Demo

IM

Screenshot 2023-07-11 193915 Screenshot 2023-07-11 194621

ROOM

Screenshot 2023-07-11 212407 Screenshot 2023-07-11 212525 Screenshot 2023-07-11 212704

Extreme

Screenshot 2023-07-11 213440

Redesigned Demo Page 😀

image

Signed-off-by: Kiran Niranjan <kiran.niranjan@symphony.com>
@KiranNiranjan KiranNiranjan self-assigned this Jul 11, 2023
@NguyenTranHoangSym NguyenTranHoangSym changed the title SDA-4420 (Implement API for call notification) SDA-4220 (Implement API for call notification) Jul 12, 2023
Signed-off-by: Kiran Niranjan <kiran.niranjan@symphony.com>
Signed-off-by: Kiran Niranjan <kiran.niranjan@symphony.com>
Copy link
Contributor

@sbenmoussati sbenmoussati left a comment

Choose a reason for hiding this comment

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

Impressive work! 👏
Minor comments to support some unexpected behaviours and give more flexibility

parseInt(String(y), 10),
);
} catch (err) {
console.warn(
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
console.warn(
logger.warn(

let containerCssClass = `container ${themeClassName} `;
containerCssClass += customCssClasses.join(' ');

const acceptText = acceptButtonText
Copy link
Contributor

Choose a reason for hiding this comment

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

What about giving full flexibility on the wording? --> sent through the API?
Logic could be implemented on client side, giving more flexibility.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, acceptButtonText if present will be used as the button text else it takes the chat type to construct the text

@@ -1,10 +1,71 @@
<html>
<head>
<style>
body {
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

Signed-off-by: Kiran Niranjan <kiran.niranjan@symphony.com>
…hange

Signed-off-by: Kiran Niranjan <kiran.niranjan@symphony.com>
@KiranNiranjan KiranNiranjan merged commit dcbe7a0 into finos:main Jul 16, 2023
5 of 6 checks passed
@KiranNiranjan KiranNiranjan deleted the SDA-4220 branch July 16, 2023 05:34
NguyenTranHoangSym pushed a commit to NguyenTranHoangSym/SymphonyElectron that referenced this pull request Sep 5, 2023
* SDA-4420 - Implement API for call notification

Signed-off-by: Kiran Niranjan <kiran.niranjan@symphony.com>

* SDA-4420 - Add unit tests

Signed-off-by: Kiran Niranjan <kiran.niranjan@symphony.com>

* SDA-4420 - Fix caller name style

Signed-off-by: Kiran Niranjan <kiran.niranjan@symphony.com>

* SDA-4420 - Change to logger

Signed-off-by: Kiran Niranjan <kiran.niranjan@symphony.com>

* SDA-4420 - update call toast position based on notification setting change

Signed-off-by: Kiran Niranjan <kiran.niranjan@symphony.com>

---------

Signed-off-by: Kiran Niranjan <kiran.niranjan@symphony.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants