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.

Sign up for GitHub

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

[Stats Loading] Display cached data if there's no connection #11654

Merged
merged 7 commits into from
May 10, 2019
Merged

[Stats Loading] Display cached data if there's no connection #11654

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5852f81
Don’t display the offline if the feature flag is on
danielebogo May 8, 2019
f719157
Merge branch 'develop' into issues/10840-offline-view
danielebogo May 9, 2019
5776880
Fix add subview
danielebogo May 9, 2019
2c9f970
If there’s no connection force to display the image
danielebogo May 9, 2019
2df6995
Remove out of scope code
danielebogo May 9, 2019
7ae9a3f
Check the reachability to hide / display the image
danielebogo May 9, 2019
a29820b
Merge branch 'develop' into issues/10840-offline-view
danielebogo May 10, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions WordPress/Classes/ViewRelated/Blog/NoResultsViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,11 @@ import Reachability
/// Public method to hide/show the image view.
///
@objc func hideImageView(_ hide: Bool = true) {
if let isReachable = reachability?.isReachable(),
Copy link
Contributor

@ScoutHarris ScoutHarris May 9, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Hey @danielebogo . I'm a little concerned about putting this here. hideImageView is a public method that is called to essentially force the image to hide without question. A cursory search shows this is typically done based on device orientation, especially for iPhone landscape where there isn't a lot of space. On the views that expect the image to be hidden, it will instead be shown for no connection, which probably won't look very nice.

Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah! Good catch! Yes in that case I have to remove that.
I have a question about this. In func configureView(), line 311, can the imageView.image be nil by default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nevermind @ScoutHarris ! I came up with another solution!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fixed it in 7ae9a3f.
You were right and that check was out of scope.

!isReachable {
hideImage = false
return
}
hideImage = hide
}

Expand Down