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

Show release notes UI in inbox when notable new features #2473

Merged
merged 2 commits into from
Nov 23, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion Classes/Issues/IssuesViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,8 @@ IssueManagingContextControllerDelegate {
route_push(to: IssuePreviewViewController(
markdown: messageView.text,
owner: model.owner,
repo: model.repo
repo: model.repo,
title: Constants.Strings.preview
))
}

Expand Down
9 changes: 7 additions & 2 deletions Classes/Issues/Preview/IssuePreviewViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,18 @@ final class IssuePreviewViewController: UIViewController, ListAdapterDataSource
init(
markdown: String,
owner: String,
repo: String
repo: String,
title: String,
asMenu: Bool = false
) {
self.markdown = markdown
self.owner = owner
self.repo = repo
super.init(nibName: nil, bundle: nil)
title = NSLocalizedString("Preview", comment: "")
self.title = title
if asMenu {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure what this does/when it is used? What is this ViewController showing again?

Copy link
Member Author

Choose a reason for hiding this comment

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

Previewing markdown

preferredContentSize = Styles.Sizes.contextMenuSize
}
}

required init?(coder aDecoder: NSCoder) {
Expand Down
98 changes: 98 additions & 0 deletions Classes/Notifications/NewFeaturesCell.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
//
// NewFeaturesCell.swift
// Freetime
//
// Created by Ryan Nystrom on 11/22/18.
// Copyright © 2018 Ryan Nystrom. All rights reserved.
//

import UIKit
import SnapKit

protocol NewFeaturesCellDelegate: class {
func didTapClose(for cell: NewFeaturesCell)
}

final class NewFeaturesCell: UICollectionViewCell {

static let inset = UIEdgeInsets(
top: Styles.Sizes.rowSpacing,
left: Styles.Sizes.gutter,
bottom: Styles.Sizes.rowSpacing,
right: Styles.Sizes.gutter
)

weak var delegate: NewFeaturesCellDelegate?

private let label = UILabel()
Copy link
Collaborator

Choose a reason for hiding this comment

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

This could be a bit more descriptive, I think? 😄

private let closeButton = UIButton()

override init(frame: CGRect) {
super.init(frame: frame)

isAccessibilityElement = true

backgroundColor = .white
contentView.backgroundColor = Styles.Colors.Blue.light.color
contentView.layer.cornerRadius = Styles.Sizes.cardCornerRadius
contentView.layer.borderWidth = 1 / UIScreen.main.scale
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be 1 * scale? Not sure what you're trying to achieve here.

Copy link
Member

Choose a reason for hiding this comment

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

Looks to be a 1px thin line rather than a 1pt thin line! I've used this a couple times 😄

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, a borderWidth of 1 would be in points. Interesting 🤔

contentView.layer.borderColor = Styles.Colors.blueGray.color.cgColor

contentView.addSubview(label)
contentView.addSubview(closeButton)

let tint = Styles.Colors.Blue.medium.color
label.textColor = tint
label.font = Styles.Text.secondaryBold.preferredFont
label.isAccessibilityElement = false

let closeButtonSize = Styles.Sizes.icon.width
closeButton.setImage(UIImage(named: "x-small")?.withRenderingMode(.alwaysTemplate), for: .normal)
closeButton.tintColor = tint
closeButton.layer.cornerRadius = closeButtonSize / 2
closeButton.addTarget(self, action: #selector(onCloseButton), for: .touchUpInside)
closeButton.accessibilityLabel = NSLocalizedString("Dismiss new feature modal", comment: "")

label.snp.makeConstraints { make in
make.centerY.equalToSuperview()
make.left.equalTo(Styles.Sizes.gutter)
make.right.lessThanOrEqualTo(closeButton.snp.left).offset(-Styles.Sizes.columnSpacing)
}
closeButton.snp.makeConstraints { make in
make.centerY.equalToSuperview()
make.right.equalTo(-Styles.Sizes.gutter)
make.size.equalTo(closeButtonSize)
}
}

required init?(coder aDecoder: NSCoder) {
fatalError("init(coder:) has not been implemented")
}

override func layoutSubviews() {
super.layoutSubviews()
contentView.frame = UIEdgeInsetsInsetRect(CGRect(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this the same as inset(by:)? That'd be a bit more Swifty and readable :)

Copy link
Member Author

Choose a reason for hiding this comment

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

That’s for x and y insetting (not UIEdgeInsets)

Copy link
Member Author

Choose a reason for hiding this comment

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

oh TIL!

Copy link
Member Author

Choose a reason for hiding this comment

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

Swift 4.2 (codebase still 4.0)

screen shot 2018-11-23 at 11 23 52 am

Copy link
Collaborator

Choose a reason for hiding this comment

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

:D

x: safeAreaInsets.left,
y: bounds.minY,
width: bounds.width - safeAreaInsets.left - safeAreaInsets.right,
height: bounds.height
), NewFeaturesCell.inset)
}

@objc private func onCloseButton() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we also add an accessibilityPerformEscape() here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I’m actually not sure what that is, but this probably needs better AX

Copy link
Collaborator

@BasThomas BasThomas Nov 23, 2018

Choose a reason for hiding this comment

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

It lets you use a z-swipe to dismiss the view controller.

delegate?.didTapClose(for: self)
}

func configure(with version: String) {
label.text = String.localizedStringWithFormat(
NSLocalizedString("New features in %@!", comment: ""),
version
)
}

override func accessibilityPerformEscape() -> Bool {
Copy link
Collaborator

Choose a reason for hiding this comment

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

💪

delegate?.didTapClose(for: self)
return true
}

}
77 changes: 77 additions & 0 deletions Classes/Notifications/NewFeaturesController.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
//
// NewFeaturesController.swift
// Freetime
//
// Created by Ryan Nystrom on 11/22/18.
// Copyright © 2018 Ryan Nystrom. All rights reserved.
//

import Foundation
import GitHubAPI
import IGListKit

final class NewFeaturesController {

// change to true for hardcoded testing
private let testing = false

var version: String {
if testing {
return "test"
}
return Bundle.main.infoDictionary?["CFBundleShortVersionString"] as? String ?? ""
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pretty sure this is safe; maybe use implicitly unwrapped optionals or an assert?

Copy link
Member Author

Choose a reason for hiding this comment

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

Want to avoid any implicit unwraps for stuff that can’t be tested. If this changed out from under us it’d stink (however unlikely).

Copy link
Collaborator

Choose a reason for hiding this comment

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

An assert would be perfect then?

}

private var fetchURL: URL? {
return URLBuilder(host: "raw.githubusercontent.com")
.add(paths: ["GitHawkApp", "Release-Notes", "master", "versions", "\(version).md"])
.url
}

private let session: URLSession = {
let config = URLSessionConfiguration.default
config.requestCachePolicy = .reloadIgnoringLocalCacheData
config.urlCache = nil
return URLSession.init(configuration: config)
}()

private let userDefaultsKey = "com.freetime.new-features-controller.has-fetched"

private var hasFetchedLatest: Bool {
get {
guard let last = UserDefaults.standard.string(forKey: userDefaultsKey) else {
return false
}
// value kept to latest version when seen
// pinned to old versions (or empty) when out of date
return last >= version
}
set {
if newValue {
UserDefaults.standard.set(version, forKey: userDefaultsKey)
} else {
UserDefaults.standard.removeObject(forKey: userDefaultsKey)
}
}
}

func fetch(success: @escaping (String) -> Void) {
guard let url = fetchURL,
(testing == true || hasFetchedLatest == false)
else { return }
hasFetchedLatest = true

let task = session.dataTask(with: url) { (data, response, _) in
if let response = response as? HTTPURLResponse,
response.statusCode == 200,
let data = data,
let string = String(data: data, encoding: .utf8) {
DispatchQueue.main.async {
success(string)
}
}
}
task.resume()
}

}
74 changes: 74 additions & 0 deletions Classes/Notifications/NewFeaturesSectionController.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
//
// NewFeaturesSectionController.swift
// Freetime
//
// Created by Ryan Nystrom on 11/22/18.
// Copyright © 2018 Ryan Nystrom. All rights reserved.
//

import Foundation
import IGListKit
import ContextMenu

final class NewFeaturesSectionController: ListSwiftSectionController<String>,
NewFeaturesCellDelegate {

private var markdown: String?
private let controller = NewFeaturesController()
private var closed = false
Copy link
Collaborator

Choose a reason for hiding this comment

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

What kind of state does this represent? And super-nit: isClosed?

Copy link
Member Author

Choose a reason for hiding this comment

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

If the user tapped the X to dismiss


override init() {
super.init()
controller.fetch { [weak self] markdown in
self?.markdown = markdown
self?.update()
}
}

override func createBinders(from value: String) -> [ListBinder] {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is some... interesting code. Having difficulties parsing/understanding this completely :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Takes some getting used to. Heavy use of generics but result is completely type safe.

guard let markdown = self.markdown,
closed == false
else { return [] }
return [
binder(
markdown,
cellType: ListCellType.class(NewFeaturesCell.self),
size: {
return $0.collection.cellSize(
with: Styles.Sizes.tableCellHeight
+ NewFeaturesCell.inset.top
+ NewFeaturesCell.inset.bottom
)
},
configure: { [weak self] (cell, _) in
guard let `self` = self else { return }
cell.configure(with: self.controller.version)
cell.delegate = self
},
didSelect: { [weak self] _ in
self?.showChanges(markdown: markdown)
})
]
}

private func showChanges(markdown: String) {
viewController?.showContextualMenu(IssuePreviewViewController(
markdown: markdown,
owner: "GitHawkApp",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think someone added the GitHawk owner/repo combination in a constant somewhere? Not sure who/where.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@BasThomas Yup I did! But its in the Templates pull request which hasn't been merged

repo: "GitHawk",
title: String.localizedStringWithFormat(
NSLocalizedString("New in %@", comment: ""),
controller.version
),
asMenu: true
))
}

// MARK: NewFeaturesCellDelegate

func didTapClose(for cell: NewFeaturesCell) {
closed = true
update()
}

}
10 changes: 9 additions & 1 deletion Classes/Notifications/NotificationsViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -262,13 +262,21 @@ BaseListViewControllerEmptyDataSource {
// MARK: BaseListViewControllerDataSource

func models(adapter: ListSwiftAdapter) -> [ListSwiftPair] {
return notificationIDs.compactMap { id in
var models = [ListSwiftPair]()

models.append(ListSwiftPair.pair("com.freetime.notification.new-features", {
NewFeaturesSectionController()
}))

models += notificationIDs.compactMap { id in
guard let model = modelController.githubClient.cache.get(id: id) as NotificationViewModel?
else { return nil }
return ListSwiftPair.pair(model) { [modelController] in
NotificationSectionController(modelController: modelController)
}
}

return models
}

// MARK: BaseListViewControllerEmptyDataSource
Expand Down
1 change: 1 addition & 0 deletions Classes/Views/Constants.swift
Original file line number Diff line number Diff line change
Expand Up @@ -51,5 +51,6 @@ enum Constants {
static let assignees = NSLocalizedString("Assignees", comment: "")
static let reviewers = NSLocalizedString("Reviewers", comment: "")
static let clear = NSLocalizedString("Clear", comment: "")
static let preview = NSLocalizedString("Preview", comment: "")
}
}
3 changes: 2 additions & 1 deletion Classes/Views/IssueTextActionsView+Markdown.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ extension IssueTextActionsView {
viewController?.route_push(to: IssuePreviewViewController(
markdown: getMarkdownBlock(),
owner: owner,
repo: repo
repo: repo,
title: Constants.Strings.preview
))
}),
name: NSLocalizedString(
Expand Down
14 changes: 13 additions & 1 deletion Freetime.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,9 @@
299304691FBA8A88007B9737 /* IssueManagingSectionController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 299304681FBA8A88007B9737 /* IssueManagingSectionController.swift */; };
2993046B1FBA8C04007B9737 /* IssueManagingExpansionCell.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2993046A1FBA8C04007B9737 /* IssueManagingExpansionCell.swift */; };
2993046F1FBA9D31007B9737 /* IssueManagingActionCell.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2993046E1FBA9D31007B9737 /* IssueManagingActionCell.swift */; };
2996543621A73885006B2CA9 /* NewFeaturesCell.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2996543521A73885006B2CA9 /* NewFeaturesCell.swift */; };
2996543C21A76F3A006B2CA9 /* NewFeaturesSectionController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2996543B21A76F3A006B2CA9 /* NewFeaturesSectionController.swift */; };
2996544021A7B14E006B2CA9 /* NewFeaturesController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2996543F21A7B14E006B2CA9 /* NewFeaturesController.swift */; };
29973E561F68BFDE0004B693 /* Signature.swift in Sources */ = {isa = PBXBuildFile; fileRef = 29973E551F68BFDE0004B693 /* Signature.swift */; };
2999972620310E9700995FFD /* IssueMergeModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2999972520310E9700995FFD /* IssueMergeModel.swift */; };
2999972820310F3100995FFD /* IssueMergeContextModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2999972720310F3100995FFD /* IssueMergeContextModel.swift */; };
Expand Down Expand Up @@ -842,6 +845,9 @@
299304681FBA8A88007B9737 /* IssueManagingSectionController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IssueManagingSectionController.swift; sourceTree = "<group>"; };
2993046A1FBA8C04007B9737 /* IssueManagingExpansionCell.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IssueManagingExpansionCell.swift; sourceTree = "<group>"; };
2993046E1FBA9D31007B9737 /* IssueManagingActionCell.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IssueManagingActionCell.swift; sourceTree = "<group>"; };
2996543521A73885006B2CA9 /* NewFeaturesCell.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NewFeaturesCell.swift; sourceTree = "<group>"; };
2996543B21A76F3A006B2CA9 /* NewFeaturesSectionController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NewFeaturesSectionController.swift; sourceTree = "<group>"; };
2996543F21A7B14E006B2CA9 /* NewFeaturesController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NewFeaturesController.swift; sourceTree = "<group>"; };
29973E551F68BFDE0004B693 /* Signature.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Signature.swift; sourceTree = "<group>"; };
2999972520310E9700995FFD /* IssueMergeModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IssueMergeModel.swift; sourceTree = "<group>"; };
2999972720310F3100995FFD /* IssueMergeContextModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IssueMergeContextModel.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -2006,10 +2012,13 @@
isa = PBXGroup;
children = (
29F3A18720CBFF8700645CB7 /* CreateNotificationTitle.swift */,
29B5D08A20D578DB003DFBE2 /* InboxType.swift */,
2996543521A73885006B2CA9 /* NewFeaturesCell.swift */,
2996543F21A7B14E006B2CA9 /* NewFeaturesController.swift */,
2996543B21A76F3A006B2CA9 /* NewFeaturesSectionController.swift */,
290EF5751F06BA06006A2160 /* NoNewNotificationsCell.swift */,
290EF5781F06BAF4006A2160 /* NoNewNotificationsSectionController.swift */,
290EF5691F06A7E1006A2160 /* Notification+NotificationViewModel.swift */,
29B5D08A20D578DB003DFBE2 /* InboxType.swift */,
29BBD82A20CACB2F004D62FE /* NotificationCell.swift */,
29F3A18520CBF99E00645CB7 /* NotificationModelController.swift */,
29BBD82C20CACDFF004D62FE /* NotificationSectionController.swift */,
Expand Down Expand Up @@ -2827,6 +2836,7 @@
29CC294D1FF839CF006B6DE7 /* IssueCommentBaseCell.swift in Sources */,
297AE87E1EC0D5C200B44A1F /* AppDelegate.swift in Sources */,
2965F37220715161003CC92F /* StyledTextBuilder+NewBase.swift in Sources */,
2996543621A73885006B2CA9 /* NewFeaturesCell.swift in Sources */,
290744B41F250A6800FD9E48 /* AutocompleteCell.swift in Sources */,
290744B81F250A7200FD9E48 /* AutocompleteType.swift in Sources */,
291929671F3FF9C50012067B /* BadgeNotifications.swift in Sources */,
Expand Down Expand Up @@ -2880,6 +2890,7 @@
294B11241F7B37D300E04F2D /* ImageCellHeightCache.swift in Sources */,
294563EC1EE5012100DBCD35 /* Issue+IssueType.swift in Sources */,
290CA7642169799600DE04F8 /* AppController.swift in Sources */,
2996543C21A76F3A006B2CA9 /* NewFeaturesSectionController.swift in Sources */,
29AF1E821F8AAB2B0008A0EF /* EditCommentViewController.swift in Sources */,
297403D71F1851C000ABA95A /* IssueAssigneeAvatarCell.swift in Sources */,
297403D11F184F8D00ABA95A /* IssueAssigneesModel.swift in Sources */,
Expand Down Expand Up @@ -3065,6 +3076,7 @@
98647DF31F758CCF00A4DE7A /* NewIssueTableViewController.swift in Sources */,
290EF5761F06BA06006A2160 /* NoNewNotificationsCell.swift in Sources */,
D8C2AEF51F9AA94600A95945 /* DotListView.swift in Sources */,
2996544021A7B14E006B2CA9 /* NewFeaturesController.swift in Sources */,
7B4B31A61F92257400480CE9 /* SearchQueryTokenizer.swift in Sources */,
2986B3581FD30EC400E3CFC6 /* IssueManagingExpansionModel.swift in Sources */,
2999972C20311DD700995FFD /* IssueMergeSummaryCell.swift in Sources */,
Expand Down