Skip to content
This repository has been archived by the owner on Dec 14, 2021. It is now read-only.

toggle password to hidden on backgrounding #995

Merged
merged 3 commits into from
May 21, 2019
Merged

Conversation

sashei
Copy link
Contributor

@sashei sashei commented May 20, 2019

Fixes #988

Testing and Review Notes

Testing by backgrounding + returning to the app should be sufficient here.

codereviewers: I did a light refactor to make the ItemDetailStore cleaner here.

To Do

  • double check the original issue to confirm it is fully satisfied
  • add testing notes and screenshots in PR description to help guide reviewers
  • add unit tests
  • make sure CI builds are passing (e.g.: fix lint and other errors)

@sashei sashei requested a review from a team as a code owner May 20, 2019 19:45
Copy link
Contributor

@jhugman jhugman left a comment

Choose a reason for hiding this comment

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

A couple of things to think about/clarify.

lockbox-ios/Store/ItemDetailStore.swift Outdated Show resolved Hide resolved
lockbox-ios/Store/ItemDetailStore.swift Show resolved Hide resolved
lockbox-ios/Store/ItemDetailStore.swift Show resolved Hide resolved
@sashei sashei requested a review from jhugman May 21, 2019 16:59
@sashei sashei dismissed jhugman’s stale review May 21, 2019 19:39

addressed

Copy link
Contributor

@jhugman jhugman left a comment

Choose a reason for hiding this comment

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

👍 Aces.

I have a question re: MainRouteAction, but otherwise, LGTM. Well done!

.disposed(by: self.disposeBag)

self.routeStore.onRoute
.filterByType(class: MainRouteAction.self)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Aren't all Actions that come out of the RouteStore MainRouteActions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, we separated the RouteActions on iOS into LoginRouteActions, MainRouteAction etc.... :p

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.

Toggle the password to hidden every time we go to the background
2 participants