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

データを保存したとき・写真を選択したときに振動がくるよう変更 #3

Merged
merged 5 commits into from Dec 7, 2017

Conversation

Projects
None yet
2 participants
@hhyyg
Owner

hhyyg commented Dec 6, 2017

No description provided.

hhyyg added some commits Dec 6, 2017

@hhyyg hhyyg requested a review from mono0926 Dec 6, 2017

@@ -16,11 +16,18 @@ class MealViewController: UIViewController, UINavigationControllerDelegate {
@IBOutlet weak var ratingControl: RatingControl!
var meal: Meal?
let notificationFeedbackGenerator = UINotificationFeedbackGenerator()
let impactFeedbackGenerator = UIImpactFeedbackGenerator(style: .medium)

This comment has been minimized.

@mono0926

mono0926 Dec 6, 2017

Collaborator

全体的にprivate付けた方が無難
(面倒だしletで不変だから公開しちゃっても良いかという割り切りもありだけど)

This comment has been minimized.

@mono0926

mono0926 Dec 6, 2017

Collaborator

名前は、imageTappedFeedbackみたいな利用シーンを表す方が分かりやすいかも。

そもそもこれも実アプリでは別途定義がおすすめ。

screen shot 2017-12-06 at 12 20 57

photoImageView.isUserInteractionEnabled = true
photoImageView.addGestureRecognizer(tapGesture)

This comment has been minimized.

@mono0926

mono0926 Dec 6, 2017

Collaborator

configureImageTapGestureみたいな別メソッドに分ける方がきれい(スコープ狭くなるので変数名もgestureとか短くできるし)

ただ、僕もサボってここに書くこと多々。

photoImageView.addGestureRecognizer(tapGesture)
notificationFeedbackGenerator.prepare()
impactFeedbackGenerator.prepare()

This comment has been minimized.

@mono0926

mono0926 Dec 6, 2017

Collaborator

didLoadでprepareしても数秒後にidleになるらしいので、大して意味無さそうだけど一応やるのもまあありかも。(個人的にはprepare使ってない)

@hhyyg

This comment has been minimized.

Owner

hhyyg commented Dec 6, 2017

@mono0926 お願いします

  • feedback系を切り分けました。(prepareは不要とし、初期化だけあらかじめ行うように)
photoImageView.isUserInteractionEnabled = true
photoImageView.addGestureRecognizer(gesture)
}
override func didReceiveMemoryWarning() {
super.didReceiveMemoryWarning()
// Dispose of any resources that can be recreated.
}
@objc func selectImageFromPhotoLibrary(_ sender: UITapGestureRecognizer) {

This comment has been minimized.

@mono0926

mono0926 Dec 6, 2017

Collaborator

さっき指摘し忘れたけど、photoImageViewDidTapみたいな名前が良い。
その中で呼ぶメソッドがselectImageFromPhotoLibrary(メソッドとして切出さずにベタ書きで良いと思うけど)。

This comment has been minimized.

@hhyyg

hhyyg Dec 6, 2017

Owner

直しました。(チュートリアルはselect~だったり)

override func didReceiveMemoryWarning() {
super.didReceiveMemoryWarning()
// Dispose of any resources that can be recreated.
}
@objc func selectImageFromPhotoLibrary(_ sender: UITapGestureRecognizer) {
impactFeedbackGenerator.impactOccurred()
feedback.occurred(scene: .willSelectPhoto)

This comment has been minimized.

@mono0926

mono0926 Dec 6, 2017

Collaborator

feedbackはoccurredされるもの(起こったもの)ではなくこれから起こすものだから英語として不自然なような 🤔

This comment has been minimized.

@mono0926

mono0926 Dec 6, 2017

Collaborator

あ、元々impactOccurredか。ちょっと考え中 🤔

This comment has been minimized.

@mono0926

mono0926 Dec 6, 2017

Collaborator

元々のはimpactoccurred したよとgeneratorに伝えている感じだから良さげ。
impactみたいな目的語(?)が欠損して、feedbackが目的語ぽくなってて不自然な感 🤔

This comment has been minimized.

@hhyyg

hhyyg Dec 6, 2017

Owner

occurにしてみた

override func didReceiveMemoryWarning() {
super.didReceiveMemoryWarning()
// Dispose of any resources that can be recreated.
}
@IBAction func selectImageFromPhotoLibrary(_ sender: UITapGestureRecognizer) {
@objc func photoImageViewDidTap(_ sender: UITapGestureRecognizer) {
feedback.occur(scene: .willSelectPhoto)

This comment has been minimized.

@mono0926

mono0926 Dec 6, 2017

Collaborator

occurredの方がまだ良かった気がする。

僕の例みたいに安直にFeedbackGeneratorクラスのgenerateメソッドとかで良いと思うけど、それがイヤなら、

feedbackオブジェクトにsceneをXXXする、のXXXがこのメソッド名に相当するから、このあたり?

  • notify
  • publish

occurが自動詞であることを考慮すると、外部引数名(ラベル)を変えて、こんな感じにするのもまあまあ良いかも。(withであっているか自信無いけど)

feedback.occurred(with: .willSelectPhoto)

This comment has been minimized.

@hhyyg

hhyyg Dec 7, 2017

Owner

ライブラリのラッパーなだけなので、ライブラリの名前に合わせる形に変更しました

@mono0926

This comment has been minimized.

Collaborator

mono0926 commented Dec 7, 2017

default

@hhyyg

This comment has been minimized.

Owner

hhyyg commented Dec 7, 2017

14_

@hhyyg hhyyg merged commit 3989e9a into master Dec 7, 2017

@hhyyg hhyyg deleted the feedback-generator branch Dec 7, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment