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

Referrals: Send pass UI #2120

Merged
merged 14 commits into from
Sep 6, 2024
Merged

Referrals: Send pass UI #2120

merged 14 commits into from
Sep 6, 2024

Conversation

SergioEstevao
Copy link
Contributor

@SergioEstevao SergioEstevao commented Sep 3, 2024

| 📘 Part of: #2083 |
|:---:|

Fixes #

This PR implements the UI only for the Send Pass UI for Referrals.

It still does not implement the background animated gradient on the Cards. That will be done a separate PR.

1 2 3
Simulator Screenshot - iPhone 15 Pro - 2024-09-04 at 16 54 06 Simulator Screenshot - iPad mini (6th generation) - 2024-09-04 at 16 56 39 Simulator Screenshot - iPad mini (6th generation) - 2024-09-04 at 16 56 24

To test

  1. Start the app
  2. Ensure that you have the Transcripts FF enabled and you logged with a Plus or Patron account
  3. Go to Profile View Controller
  4. Tap on the gift icon on the top left
  5. Check if the new guest pass screen is show
  6. Tap on the close button
  7. Tap on the share guest pass button
  8. Test on the iPad

Checklist

  • I have considered if this change warrants user-facing release notes and have added them to CHANGELOG.md if necessary.
  • I have considered adding unit tests for my changes.
  • I have updated (or requested that someone edit) the spreadsheet to reflect any new or changed analytics.

@SergioEstevao SergioEstevao added this to the 7.73 milestone Sep 3, 2024
@SergioEstevao SergioEstevao marked this pull request as ready for review September 4, 2024 16:00
@SergioEstevao SergioEstevao requested a review from a team as a code owner September 4, 2024 16:00
@SergioEstevao SergioEstevao requested review from bjtitus and removed request for a team September 4, 2024 16:00
Copy link
Contributor

@bjtitus bjtitus left a comment

Choose a reason for hiding this comment

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

Looked good in testing on both iPhone and iPad.

Left a small note but I'm not really sure about the whole Constants thing in SwiftUI TBH.

.renderingMode(.template)
.resizable()
.aspectRatio(contentMode: .fill)
.frame(width: 12, height: 12)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be in Constants? I'm not really sure I like it any better having so many Constants and just jumping between lines but I noticed the defaultCardSize defined as a constant in another file.l

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed here: 2b0689f

For the record I'm also a bit split when to use the enum Constants or not, I like to use them when the value is used in multiplace spaces or to make clear the intention of the value.

@SergioEstevao SergioEstevao merged commit e5e904b into trunk Sep 6, 2024
4 of 6 checks passed
@SergioEstevao SergioEstevao deleted the referrals/send_pass_ui branch September 6, 2024 13:04
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.

2 participants