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

Removing drop shadow for transparent In App Messages #1078

Merged
merged 1 commit into from
Apr 21, 2022

Conversation

emawby
Copy link
Contributor

@emawby emawby commented Apr 20, 2022

Description

One Line Summary

Removing the drop shadow for elements in In App Messages with transparent backgrounds.

Details

This dropshadow only appeared when the background of the IAM was transparent. This is not an intentional visual feature so we are removing it. The shadow was added in the initial implementation of IAMs and was likely never noticed since it only appears for transparent backgrounds.

Motivation

Bug noticed when implementing transparent IAM backgrounds.

Scope

transparent background iOS IAMs

Testing

Unit testing

no unit tests for this visual change

Manual testing

Tested on iOS device and simulator as well as Android to verify this only affects iOS

Affected code checklist

  • Notifications
    • Display
    • Open
    • Push Processing
    • Confirm Deliveries
  • Outcomes
  • Sessions
  • In-App Messaging
  • REST API requests
  • Public API changes

Checklist

Overview

  • I have filled out all REQUIRED sections above
  • PR does one thing
    • If it is hard to explain how any codes changes are related to each other then it most likely needs to be more than one PR
  • Any Public API changes are explained in the PR details and conform to existing APIs

Testing

  • I have included test coverage for these changes, or explained why they are not needed
  • All automated tests pass, or I explained why that is not possible
  • I have personally tested this on my device, or explained why that is not possible

Final pass

  • Code is as readable as possible.
    • Simplify with less code, followed by splitting up code into well named functions and variables, followed by adding comments to the code.
  • I have reviewed this PR myself, ensuring it meets each checklist item
    • WIP (Work In Progress) is ok, but explain what is still in progress and what you would like feedback on. Start the PR title with "WIP" to indicate this.

This change is Reviewable

This dropshadow only appeared when the background of the IAM was transparent. This is not an intentional visual feature so we are removing it.
@emawby emawby requested review from a team, jkasten2 and nan-li April 20, 2022 21:31
@jkasten2
Copy link
Member

Found the original commit this code was added in.
e4b4e76#diff-ec01572d99b228c79233db455f9e18254ef5b18bfbd5c9f4e3e09f820d20acfaR112-R116

  • All IAMs now have corners and drop shadows for a better look

Copy link
Member

@jkasten2 jkasten2 left a comment

Choose a reason for hiding this comment

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

Side-by-Side screenshot would be nice in the PR description for a visual change.

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @nan-li)

Copy link
Member

@jkasten2 jkasten2 left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @nan-li)

@emawby emawby merged commit e4eebb0 into main Apr 21, 2022
@emawby emawby deleted the fix/remove_dropshadow_for_transparent_IAMs branch April 21, 2022 15:31
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.

None yet

2 participants