-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[MBL-1783 Unhide rewards immediately if there's no shippable rewards #2178
Conversation
guard let self else { return } | ||
|
||
self.collectionView.isHidden = !shimmerLoadingViewIsHidden | ||
self.collectionView.layoutIfNeeded() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I deleted this because now that we're not always waiting for the location view to load, it gets called before the view hierarchy is ready. I did test a bunch of random projects (hence the local pickup bug I found) and didn't see any UI weirdness caused by removing this line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't really be sure that this behavior is fully correct as I haven't been in this area much, but changes seem to make sense to me!
guard let self else { return } | ||
|
||
self.collectionView.isHidden = !shimmerLoadingViewIsHidden | ||
self.collectionView.layoutIfNeeded() | ||
self.collectionView.isHidden = rewardsCollectionViewIsHidden |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Making sure the intent was to invert this isHidden check
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I found the naming to be super confusing before, so I've inverted it (both here and in the view model) to be more intuitive. It's still in sync with the underlying value; it's just the output signal value that's inverted. Thanks for checking though!
π² What
We currently hide all rewards until the shipping location selector is done loading. If there aren't any shippable rewards, unhide the rewards immediately.
π See
Jira
β Acceptance criteria