Default recommended projects #680
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* New card screen ui (#471) * add card view controller * transition to add new card and uibarbuttons * settings style applied to add new card screen * changed keyboard style for textfields * view controller tests * placeholder for cardholder name textfield * Record new screenshots * new strings, added strings as place holder and text label * pr feedback - inferring type for cancel button tintcolor * pr feedback - keypath, no more lenses * swiftlint fix * new snapshots * Payment methods (#457) * Stripe element add new card (#473) * New card screen ui (#471) * add card view controller * transition to add new card and uibarbuttons * settings style applied to add new card screen * changed keyboard style for textfields * view controller tests * placeholder for cardholder name textfield * Record new screenshots * new strings, added strings as place holder and text label * pr feedback - inferring type for cancel button tintcolor * pr feedback - keypath, no more lenses * swiftlint fix * new snapshots * new stppaymentcardtextfield showing * wip - placeholder colors * snapshot tests * Email undeliverable/unverified (#478) * Adding emailIsVerified * Renaming GraphUserEmail * ViewModel tests * ChangeEmailViewController tests * Strings, swiftlint * Strings & new screenshots * Cleanup * PR comments * PR updates * Test naming * pr feedback * new snapshots * new placeholder colors * new snapshots * Delete payment methods (#479) * Add payment method deletion * Test datasource * Test view model * Clean-up and address PR feedback * Rename credit card -> payment method * Fix test * Add credit card implementation (#503) * Email undeliverable/unverified (#478) * Adding emailIsVerified * Renaming GraphUserEmail * ViewModel tests * ChangeEmailViewController tests * Strings, swiftlint * Strings & new screenshots * Cleanup * PR comments * PR updates * Test naming * Use correct function to log events in crash logs (#481) * Use correct function to log events in crash logs * Fix indentation * changed ui colors for textfield text and font size of text label * vm and work on enabling save * wip- get stripe token * wip- payment source mutation * wip- keyboard response * wip- getting a stripe error here * wip- error fix w/ publishable key, error banner showing * wip- saving with error and keyboard functionality * wip- updating card immediately * wip * wip - ACs met * wip - refactor in view model, made IDs testable, begane VM tests * wip -refactor on vm/vmtest * wip -refactor deleted comments on vmt * wip -snapshot tests * swiftlint fixes * renaming/refactor, corrected paymentmethodstests * pr feedback * swiftlint fixes * changed function name * indentation * Payment methods event tracking (#496) * Reverted code that deleted SettingsNewsletters from Storyboard.swift * swiftlint * Reverted code to instantiate settings newsletters vc on tests * Alphabetized storyboard enum * Settings payments colors (#530) * Unsupported Credit Cards (#561) * Viewmodel logic for unsupported cards * Tests and accessibility fixes * Simplify add card button * Screenshots and string updates * Cleanup design and remove logs * Improving comment message * Removing filter debug builds * Addressing PR comments * Updating payment methods screenshots * Screenshots * Possible fix for alignment issues * Moving iOS 10 handling to output signal closure * Remove line * Renaming * Last cleanup * Strings & Asset update * Switching from unsupported to supported * Zipcode field in Add New Card screen (#566) * Viewmodel logic for unsupported cards * Tests and accessibility fixes * Simplify add card button * Screenshots and string updates * Cleanup design and remove logs * Improving comment message * Removing filter debug builds * Addressing PR comments * Updating payment methods screenshots * Screenshots * Possible fix for alignment issues * Moving iOS 10 handling to output signal closure * Remove line * Renaming * Last cleanup * Shared styled form field * Adding functionality for zipcode form field * More functionality * View model & screenshot tests * Swiftlint * Cleanup * Improving test and cleanup * PR feedback and autocapitalizing zipcode * Regenerating ChangePassword screenshots * [Payment methods] CVC bug fix (#574) * wip * wip * wip * wip-swiftlint * moved CreatePaymentSourceEnvelope to its own file, Mockservice * swiftlint fix * deleted unnecesary debugging code * new paymentsource temploate and vm tests * swiftlint fix * fixed vm test * PR feedback * update schema * [Payment methods] Minor bug fixes (#579) * tableViewIsEditing false and show banner after dismiss * Swiftlint * Adding StripePublishableKey to example file * Update publishableKey test * Spacing * Point-free helper * [Payment Methods] Update padding and image view size (#581) * Update padding and image view size * Update stack view's spacing * Update snapshots * Use layoutMargins to set the padding * [Payment methods] Disable edit button if no payment methods (#586) * [Payment methods] Bugs & visual fixes (#578) * Adding custom UITableViewHeader and always calling reload data on viewDidLoad * Updating screenshots * Cleanup autolayout warnings * Screenshots * PR comments * No autolayout for footer view * Cleanup * Remove intrinsicContentSize * Tableview appears without delay fix (#587) * Fix header/footer auto sizing * Update header background color * Update snapshots * Screenshots again * Updating constraint priority to resolve ambiguity * [Payment methods] Refresh payment methods table view properly (#588) * Refresh payment methods table view properly * Remove reference to weak self * Rename delegate methods * [Payment methods] Refetch payment methods on viewDidLoad or explicitly (by delegate) (#591) * Do not refresh on viewWillAppear * Use optional string to avoid unnecessary initializer * Bring back viewWillAppear signal to better reflect view controller's lifecycle state * [Payment methods] A11y - Credit card name (#593) * Add card name to the a11y label * Add comment * [Payment Methods] Design Fixes (#592) * Design fixes * Fresh screenshots ✨ * Remove recordMode * Deleting stale screenshots * Re-add accidentally deleted line * update strings * [Payment Methods] Support optional card type and error handling for payment methods (#596) * Optional card type, payment methods load error * Optional support for imageName * Fix accessibility label * Fix merge conflict spacing * Swiftlint * Implementing PR suggestions * Fixed bug that shows wrong expiration date (#595) * [Payment Methods] Add New Card Design Fixes (#594) * error message present and return key to done * removed error message banner for incomplete payment details * new snapshots for correct insets * [Payment methods] Optimistically disable edit button on card deletion (#589) * Disable edit button optimistically, use stored cards in result from card deletion * Fix issue where edit button is not re-enabled upon the failed deletion of the last card. * Remove unnecessary .init * Add skipRepeats() to editButtonIsEnabled signal. * Fixing potential reference cycles (#598)
1 similar comment
dusi
reviewed
May 21, 2019
There was a problem hiding this comment.
Looks good @Scollaco
I only had one question (around grammar) and one observation which I'm not sure how hard it would be to fix this but it looks like that if you're logged in and you have selected Recommended in discovery tab and you logout, then you're still in Recommended (which should no longer be available)..wonder if we can fix this edge case?
Steps to reproduce:
- Log in as a user who Opted In for recommendations
- Go to Explore and select Recommended
- Go to Settings and Logout
- Go back to Explore and you'll notice that Recommended is still selected (even though it should now not be accessible)
dusi
reviewed
May 22, 2019
There was a problem hiding this comment.
Had couple minor questions and noticed one issue with the notification:
Toggling Recommendations ON/OFF in privacy settings dismisses the Settings modal
Steps to reproduce:
- Go to Settings > Privacy
- Toggle
RecommendationsON or OFF (doesn't really matter) - User flow gets interrupted by
Settingsmodal being dismissed without any animation
justinswart
reviewed
May 23, 2019
SwiftFormat found issues:
Generated by 🚫 Danger |
Scollaco
force-pushed
the
default-recommended-projects
branch
from
7565c3d
to
f45483d
Compare
justinswart
reviewed
May 24, 2019
|
Hey I think the only thing left to figure out is why this generated so many new snapshots? |
Scollaco
force-pushed
the
default-recommended-projects
branch
from
e021e27
to
5743834
Compare
Ok, I think the problem was that I'd generated snapshots using the iPhoneX simulator 🤦♂ . I reverted the commits and once I re-ran the tests using the iphone 8 simulator, all the viewController tests passed, so snapshots were no longer necessary. |
dusi
suggested changes
May 28, 2019
Co-Authored-By: Pavel Dusatko <pavel.dusatko@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📲 What
Changes Discover behavior by setting
Recommended For Youas default filter if user is logged in. If the user is logged out, the behavior shouldn't change.🤔 Why
This feature existed in the past, but due GDPR we had decided to remove it temporarily until have a better understanding of its implications.
🛠 How
But changing the
initialParamsonDiscoveryViewModelto include recommended projects if user is logged in.👀 See
✅ Acceptance criteria
All ProjectsSettings > Account > PrivacyisOFF, Discovery should still showAll Projectsas default filter.Settings > Account > PrivacyisON, Discovery should showRecommended for youas default filter.Settings > Account > Privacyand going back to Explore tab should showAll Projectseven ifRecommended for youwas previously selectedAll Projectsas default filter.