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

feat(in-app-messaging): update InAppMessagingDisplay and useInAppMessage #9088

Conversation

calebpollman
Copy link
Member

@calebpollman calebpollman commented Oct 23, 2021

Description of changes

Follow up to PR #9026.

  • add additional logging and try/catch blocks in handleAction
  • add optional chaining on content values to prevent accessing falsy values in useInAppMessage
  • add optional chaining to call of onActionCallback in onPress passed to buttons
  • handle falsy values and prevent assigning empty values in getContentProps
  • migrate to default and explicit exports in in-app messaging feature code

Issue #, if available

NA

Description of how you validated changes

Manually tested in sample app

Checklist

  • PR description included
  • yarn test passes
  • Tests are changed or added
  • Relevant documentation is changed or added (and PR referenced)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@codecov-commenter
Copy link

codecov-commenter commented Oct 23, 2021

Codecov Report

Merging #9088 (fe38553) into in-app-messaging/main (f5ff575) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@                  Coverage Diff                   @@
##           in-app-messaging/main    #9088   +/-   ##
======================================================
  Coverage                  78.00%   78.00%           
======================================================
  Files                        250      250           
  Lines                      18115    18115           
  Branches                    3885     3885           
======================================================
  Hits                       14131    14131           
  Misses                      3854     3854           
  Partials                     130      130           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f5ff575...fe38553. Read the comment docs.

@calebpollman calebpollman requested review from cshfang, ericclemmons and nickarocho and removed request for Ashish-Nanda October 23, 2021 00:58
Copy link
Contributor

@elorzafe elorzafe left a comment

Choose a reason for hiding this comment

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

LGTM 🌮

Copy link
Contributor

@nickarocho nickarocho left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks, Caleb 🚢

@calebpollman calebpollman merged commit 64a4090 into aws-amplify:in-app-messaging/main Oct 27, 2021
@calebpollman calebpollman deleted the in-app-messaging/in-app-display-follow-up branch October 27, 2021 16:29
@github-actions
Copy link

This pull request has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels or Discussions for those types of questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants