Skip to content

Conversation

@Enuviel
Copy link
Contributor

@Enuviel Enuviel commented Jan 24, 2018

feat(BigPictureCustomizer): Adds support for modification of extended layout in BigPictureStyle with 2 lines of text.

@Enuviel Enuviel requested a review from benmarten January 24, 2018 02:41
* push notification. Call bigPictureStyle.bigLargeIcon(largeIconImage) if you want to set large
* icon on extended push notification. This bigPictureStyle will be set to push notification. Note
* that if you call bigPictureStyle= new Notification.BigPictureStyle() - there will be no support
* 2 lines of text and we will show yours extended layout in BigPicture style push notification.
Copy link
Contributor

Choose a reason for hiding this comment

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

show your extended

* @param bigPictureStyle - Notification.BigPictureStyle or null - BigPicture style for current
* push notification. Call bigPictureStyle.bigLargeIcon(largeIconImage) if you want to set large
* icon on extended push notification. This bigPictureStyle will be set to push notification. Note
* that if you call bigPictureStyle= new Notification.BigPictureStyle() - there will be no support
Copy link
Contributor

Choose a reason for hiding this comment

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

bigPictureStyle = new Notification.BigPictureStyle()

* 2 lines of text and we will show yours extended layout in BigPicture style push notification.
*/
void customize(Notification.Builder builder, Bundle notificationPayload);

Copy link
Contributor

Choose a reason for hiding this comment

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

empty line ;)

* @param notificationPayload Bundle notification payload.
* @param bigPictureStyle - Notification.BigPictureStyle or null - BigPicture style for current
* push notification. Call bigPictureStyle.bigLargeIcon(largeIconImage) if you want to set large
* icon on extended push notification. This bigPictureStyle will be set to push notification. Note
Copy link
Contributor

Choose a reason for hiding this comment

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

expanded


/**
* Calls setStyle for notificationBuilder and he tries modify notification layout for support 2
* lines.
Copy link
Contributor

Choose a reason for hiding this comment

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

it tries modify notification layout to support two

message);
} else {
LeanplumPushService.notificationBuilderCustomizer.customize(notificationBuilder, message);
Notification.BigPictureStyle bigPictureStyle;
Copy link
Contributor

Choose a reason for hiding this comment

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

one line

@Enuviel Enuviel changed the title feat(pusCustomizer): Adds support for modification of extended layout… feat(BigPictureCustomizer): Adds support for modification of extended layout… Jan 24, 2018
@Enuviel
Copy link
Contributor Author

Enuviel commented Jan 25, 2018

test this please

1 similar comment
@Enuviel
Copy link
Contributor Author

Enuviel commented Jan 25, 2018

test this please

… layout in BigPictureStyle with 2 lines of text.
Copy link
Contributor

@benmarten benmarten left a comment

Choose a reason for hiding this comment

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

please check comments

}
LeanplumPushService.customizer = customizer;
setCustomizer(customizer, false);

Copy link
Contributor

Choose a reason for hiding this comment

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

empty line

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, I am wondering if we should deprecate it now after all?
It seems to me that this is the method that most clients would want to use now?

Should we also flip the default value to setCustomizer(customizer, false) because then clients would get benefit for 2 lines support, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, right

Copy link
Contributor

@benmarten benmarten left a comment

Choose a reason for hiding this comment

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

please check comments

@Enuviel
Copy link
Contributor Author

Enuviel commented Jan 26, 2018

test this please

fischjup
fischjup approved these changes Jan 26, 2018
Copy link
Contributor

@benmarten benmarten left a comment

Choose a reason for hiding this comment

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

this looks solid to me now! thx

@Enuviel Enuviel merged commit 882e070 into develop Jan 26, 2018
@Enuviel Enuviel deleted the feat/feat-LP-8635 branch January 26, 2018 07:45
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.

4 participants