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.

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

[feature/whats new screen] In-App changelog "What´s new screen" #572

Merged
merged 21 commits into from
Jan 22, 2020

Conversation

hosy
Copy link
Collaborator

@hosy hosy commented Dec 8, 2019

Description

If the user has auto update enabled on the iOS device, the user cannot see the AppStore changelog, what changed in the new app version.

With this new screen the user can see an In-App changelog, when the app was started the first time, after an app update.

If there was more than one app update since the last app launch, the user will see everything what's new, since the last started app version.

It is important, that this file ownCloud/Release Notes/ReleaseNotes.plist needs new content before an update was submitted to Apple.
This file also contains the image data as a base64 encoded string.

The ReleaseNotesHostViewController can later easily used to integrate an Intro or Onbording view.

Motivation and Context

Better marketing to the user.

How Has This Been Tested?

  • Install app version < 1.2
  • Launch app
  • Install app version 1.2
  • Launch app What´s new screen should appear
  • Force quite App
  • Launch app again What´s new screen should NOT appear

Screenshots (if appropriate):

Simulator Screen Shot - iPhone 11 Pro - 2019-12-08 at 11 52 12

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.

QA

Test plan:

Bugs & improvements:

hosy and others added 6 commits October 9, 2019 16:24
* [fix/fp-offline-browsing] Allow offline browsing of folders in the File Provider (#547)

* - Fix Swift and SwiftLint warnings
- Remove unused UploadsSettingsSection (was replaced by MediaUploadSettings)

* - address libzip Xcode project upgrade warning
- add research note to FileProviderExtension
- update SDK

* - Allow offline browsing of folders in the File Provider

* Revert "[fix/fp-offline-browsing] Allow offline browsing of folders in the File Provider (#547)" (#553)

This reverts commit 9a0bc93.

* Autoplay media files implemented as described in issue #59

* Added album artwork as overlay in the player

* Fixed playing next media item in BG and lock screen

- Now multiple items can be played contignuously in the background
- Now playing info in the lock screen contains artwork, title, artist info and displays correct playback timeline
- Audio can be paused / resumed from the lock screen
- Added skip controls allowing to jump 10s backwards and forwards from the play-head position in the lock screen

* Small fixes
* [fix/fp-offline-browsing] Allow offline browsing of folders in the File Provider (#547)

* - Fix Swift and SwiftLint warnings
- Remove unused UploadsSettingsSection (was replaced by MediaUploadSettings)

* - address libzip Xcode project upgrade warning
- add research note to FileProviderExtension
- update SDK

* - Allow offline browsing of folders in the File Provider

* Revert "[fix/fp-offline-browsing] Allow offline browsing of folders in the File Provider (#547)" (#553)

This reverts commit 9a0bc93.

* Fix for issue #568: Share sheet was now visible on iPad, if tableview was scrolled down, after first visible page rect
- added release notes for version 1.2
- added a datasource class and a check, if release notes should appear and which items
@hosy hosy requested review from mneuwert and jesmrec December 8, 2019 15:31
@hosy hosy self-assigned this Dec 8, 2019
hosy and others added 4 commits December 8, 2019 20:55
# Conflicts:
#	ownCloud.xcodeproj/project.pbxproj
#	ownCloud/Resources/en.lproj/Localizable.strings
…p into feature/whats-new-screen

# Conflicts:
#	.github/release_template.md
titleLabel.text = "New in ownCloud".localized
titleLabel.textAlignment = .center
titleLabel.numberOfLines = 0
titleLabel.font = UIFont.systemFont(ofSize: UIFont.systemFontSize * 1.5, weight: .bold)
Copy link
Contributor

Choose a reason for hiding this comment

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

@hosy can we use dynamic type font for this? Something like .headline style or similar.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

])

proceedButton.setTitle("Proceed".localized, for: .normal)
proceedButton.layer.cornerRadius = cornerRadius
Copy link
Contributor

Choose a reason for hiding this comment

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

@hosy I noticed that we use different corner radius values for buttons, sometimes 8.0 and sometimes 10.0. Could we unify it and e.g. provide some category on UIButton with some factory method or similar?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

now using ThemeButton

footerLabel.translatesAutoresizingMaskIntoConstraints = false
footerLabel.text = "Thank you for using ownCloud.\nIf you like our App, please leave an AppStore review.\n❤️".localized
footerLabel.numberOfLines = 0
footerLabel.font = UIFont.systemFont(ofSize: 14.0)
Copy link
Contributor

Choose a reason for hiding this comment

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

@hosy Another case for dynamic type?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

}

func releaseNotes(for version: String) -> [[String:Any]]? {
if let path = Bundle.main.path(forResource: "ReleaseNotes", ofType: "plist") {
Copy link
Contributor

Choose a reason for hiding this comment

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

@hosy Looking at code in lines 154-172, I think there is some potential to reduce code duplication...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

removed one if statement

@michaelstingl michaelstingl added this to the 1.3.0-Next milestone Dec 9, 2019
@hosy hosy requested a review from mneuwert December 9, 2019 12:41
removed version 13 from release notes
@jesmrec jesmrec changed the base branch from milestone/1.2 to milestone/1.3 January 9, 2020 08:42
@jesmrec
Copy link
Contributor

jesmrec commented Jan 9, 2020

Changed base branch to milestone/1.3

@jesmrec
Copy link
Contributor

jesmrec commented Jan 9, 2020

(1) improvement [FIXED]

I'd set the view only in case of upgrading to newest version, so that user checks the new features after upgrading.

In case of a fresh install, i'd not show this. For a new user everything in the app is new, not only the latest released features. As you mentioned in the first message, for new users i'd set a welcome wizard or onboarding/welcome view in a later issue.

what do you think?

@jesmrec
Copy link
Contributor

jesmrec commented Jan 9, 2020

(2) (improvement) [DONE]

A link to the app in App Store would be helpful here:

Screenshot 2020-01-09 at 10 01 18

@jesmrec
Copy link
Contributor

jesmrec commented Jan 9, 2020

(3) [FIXED]

  1. Install from scratch -> What's new is there
  2. Kill the app
  3. Open again

Current: What's new is displayed again
Expected: What's new is displayed only once

iPadPro v13.2
iPhoneXR v12

- fix showing release notes, after was displayed
- added app review request
- changed release notes
@hosy
Copy link
Collaborator Author

hosy commented Jan 9, 2020

@jesmrec (1), (2), (3) was fixed by my side. Request for app rating is now integrated. Changed also some release notes strings.

# Conflicts:
#	ios-sdk
#	ownCloud/Client/Viewer/DisplayHostViewController.swift
#	ownCloud/Client/Viewer/Media/MediaDisplayViewController.swift
titleLabel.translatesAutoresizingMaskIntoConstraints = false
titleLabel.setContentHuggingPriority(UILayoutPriority.defaultLow, for: NSLayoutConstraint.Axis.horizontal)

titleLabel.text = "New in ownCloud".localized
Copy link
Contributor

Choose a reason for hiding this comment

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

ownCloud word should be brandable

proceedButton.addTarget(self, action: #selector(dismissView), for: .touchUpInside)
bottomView.addSubview(proceedButton)

footerButton.setTitle("Thank you for using ownCloud.\nIf you like our App, please leave an AppStore review.\n❤️".localized, for: .normal)
Copy link
Contributor

Choose a reason for hiding this comment

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

ownCloud word should be brandable

/* Release Notes */
"Proceed" = "Proceed";
"New in ownCloud" = "New in ownCloud";
"Thank you for using ownCloud.\nIf you like our App, please leave an AppStore review.\n❤️" = "Thank you for using ownCloud.\nIf you like our App, please leave an AppStore review.\n❤️";
Copy link
Contributor

Choose a reason for hiding this comment

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

ownCloud word should be brandable

@jesmrec
Copy link
Contributor

jesmrec commented Jan 14, 2020

I keep having problems to get the what's new, so i have some questions to clarify.

The typical scenario i perform is:

  1. installing the current stable version 1.2.1 in a device , checking in Settings that everything is correct.

Screenshot 2020-01-14 at 17 27 43

  1. Go to XCode -> ownCloud target -> General -> Version, and set a higher version and build number. (1.3 as version, and incrementing in one the build number)

Screenshot 2020-01-14 at 17 25 28

  1. Run the app to upgrade it in the same device, and check the Settingsto see there the new app version and build number (they match correctly)

Screenshot 2020-01-14 at 17 31 38

I tried with and without the beta warning, but the what's new is not there in anycase.

By checking the code, here is where you check if the release notes should be displayed:

var shouldShowReleaseNotes: Bool {

I set there a break point, and the if clause in the next row always returns false, so the block returns false... at least in my code executions.

Specific questions:

  • Is it correct to upgrade only both fields mentioned in step 2.? anything else to do?
  • ipa installed must be an archived release ipa? or is it enough with debug ipa?
  • Does Beta warning anything to do with the upgrading? (i guess not)

maybe i'm missing something. I hope these details help.

@jesmrec jesmrec added Approved by QA Approved by QA Estimation - 3 (M) 3 points and removed Approved by QA Approved by QA labels Jan 17, 2020
@jesmrec jesmrec self-assigned this Jan 20, 2020
@hosy
Copy link
Collaborator Author

hosy commented Jan 20, 2020

@jesmrec please check again

@jesmrec
Copy link
Contributor

jesmrec commented Jan 21, 2020

Latest round with commit 6ee498d

(1) -> What's new is displayed when the app is installed from srcratch

(2), (3) -> done!

@hosy
Copy link
Collaborator Author

hosy commented Jan 21, 2020

@jesmrec I noticed that .isBetaBuild always exists, even if the app was installed from the scratch. The only solution to prevent showing this screen after a fresh install is, to check, if there is existing an account.
I cannot catch the following scenario:

  • User installed version 1.2.1
  • No account is configured in the app
  • User updating the app to version 1.3, with no account configured
  • App will be started
  • What's new View does not appear

This will not happen, if an account was configured before updating, but I cannot catch this case.
Please check this with the latest commit 6af51be

@jesmrec
Copy link
Contributor

jesmrec commented Jan 21, 2020

It's no problem to assume that restriction. If almost one account is attached, it's working fine now.

Check also my comment here: https://github.com/owncloud/ios-app/pull/572/files#r366388448

@hosy
Copy link
Collaborator Author

hosy commented Jan 21, 2020

@jesmrec app name is now customizable in this view

@jesmrec
Copy link
Contributor

jesmrec commented Jan 21, 2020

Approved then.

@jesmrec jesmrec added the Approved by QA Approved by QA label Jan 21, 2020
@jesmrec jesmrec self-requested a review January 21, 2020 16:49
@hosy hosy merged commit ff38f3c into milestone/1.3 Jan 22, 2020
@delete-merged-branch delete-merged-branch bot deleted the feature/whats-new-screen branch January 22, 2020 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants