Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

feature: displaying a warning before opening a block explorer link #326

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

chitowncrispy
Copy link
Contributor

Description

I added a new UIAlertController to display an alert to the user when they tap on a block explorer link. I also made a slight refactor to remove the delegate that is passed into EventDetailViewModel.swift. I replaced it with a more reactive approach to more fully embrace that style of programming within the app.

Motivation and Context

This is to address issue #125.

How Has This Been Tested?

On the iOS simulator, I went to the history page, tapped on a transaction and then tapped on both transaction hashes as well as addresses. I verified that the cancel button dismissed the alert in both cases. I also verified that the continue button took us to the correct block explorer page in both instances.

I also went to the channel page and tapped on the funding transactions. I verified that the cancel button dismissed the alert. I also verified that the continue button took us to the correct block explorer page.

Screenshots (if appropriate):

Screen Shot 2019-11-16 at 10 37 34 PM

Screen Shot 2019-11-16 at 10 37 45 PM

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@codecov-io
Copy link

codecov-io commented Nov 17, 2019

Codecov Report

Merging #326 into master will decrease coverage by 0.14%.
The diff coverage is 17.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #326      +/-   ##
==========================================
- Coverage   10.71%   10.56%   -0.15%     
==========================================
  Files         247      247              
  Lines        9803     9827      +24     
==========================================
- Hits         1050     1038      -12     
- Misses       8753     8789      +36
Impacted Files Coverage Δ
Library/Extensions/UserDefaults.swift 0% <ø> (ø) ⬆️
Library/Generated/strings.swift 6.97% <ø> (ø) ⬆️
Library/Coordinators/WalletCoordinator.swift 0% <0%> (ø) ⬆️
Library/Extensions/UIAlertController.swift 0% <0%> (ø) ⬆️
...nes/History/Detail/EventDetailViewController.swift 0% <0%> (ø) ⬆️
Library/Tests/EventDetailViewModelTests.swift 97.36% <100%> (+2.49%) ⬆️
...y/Scenes/History/Detail/EventDetailViewModel.swift 36.29% <62.5%> (-1.93%) ⬇️
...htning/Services/Misc/BlockChainHeightUpdater.swift 60.41% <0%> (-20.84%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update efa92e2...9e6c4ec. Read the comment docs.

@ottosuess
Copy link
Member

Greenwallet has an option to turn that alert off. that's pretty nice for users who don't care about that notification.

We can just store that in the UserDefaults. (https://github.com/LN-Zap/zap-iOS/blob/master/Library/Extensions/UserDefaults.swift).

Image from iOS (2)

@chitowncrispy
Copy link
Contributor Author

Screen Shot 2019-11-22 at 10 37 01 AM

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.

3 participants