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

Jump to bottom

[iOS] Add Speaker View #604

Merged
merged 7 commits into from
Jan 26, 2020
Merged

[iOS] Add Speaker View #604

merged 7 commits into from
Jan 26, 2020

Conversation

yanamura
Copy link
Contributor

@yanamura yanamura commented Jan 25, 2020 •
edited
Loading

Issue

TODO

  • show session

Overview (Required)

  • just show speaker info
  • I'll do session info on the other PR

Links

Screenshot

@ry-itto
Copy link
Member

ry-itto commented Jan 25, 2020

@yanamura
Is this finished? If not, please make this PR Draft or add [WIP] at the beginning of this PR's title. 🙏

@yanamura yanamura requested a review from ry-itto January 25, 2020 23:04
@yanamura
Copy link
Contributor Author

@ry-itto I want to split PR. Because this PR includes fix Navigation bar changes, this feature is required from other issues.

ry-itto
ry-itto requested changes Jan 26, 2020
View reviewed changes
ios-base/DroidKaigi 2020/AppDelegate.swift
UINavigationBar.appearance().tintColor = ApplicationScheme.shared.colorScheme.onPrimaryColor
UINavigationBar.appearance().backgroundColor = ApplicationScheme.shared.colorScheme.primaryColor

let backButtonBackgroundImage = #imageLiteral(resourceName: "ic_back")
Copy link
Member

Choose a reason for hiding this comment

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

👍

ios-base/DroidKaigi 2020/Session/Views/SpeakerViewController.swift Outdated
import Nuke
import UIKit

final class SpeakerViewController: UIViewController {
Copy link
Member

Choose a reason for hiding this comment

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

Please move this file to new directory. Such as $SRCROOT/Droid Kaigi/Speaker/Views.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh sorry, i misunderstood project structures

ios-base/DroidKaigi 2020/Session/Views/SpeakerViewController.swift Outdated
Comment on lines 38 to 40
override func viewWillDisappear(_ animated: Bool) {
super.viewDidAppear(animated)
}
Copy link
Member

Choose a reason for hiding this comment

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

This is unnecessary. Please remove this 🙏

@ry-itto
Copy link
Member

ry-itto commented Jan 26, 2020

@yanamura
The way to apply color for navigationBar is changed by #620 . So this PR has some conflicts. Please resolve it 🙏

@yanamura yanamura force-pushed the ios-speakerview branch 2 times, most recently from e5e73a7 to 3c5f6f2 Compare January 26, 2020 12:58
@yanamura yanamura requested a review from ry-itto January 26, 2020 13:03
@yanamura
Copy link
Contributor Author

fixed

ry-itto
ry-itto requested changes Jan 26, 2020
View reviewed changes
ios-base/DroidKaigi 2020/AppDelegate.swift
@@ -9,7 +9,11 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
if #available(iOS 13, *) {
} else {
let window = UIWindow(frame: UIScreen.main.bounds)
UINavigationBar.appearance().isTranslucent = false
Copy link
Member

Choose a reason for hiding this comment

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

Please add this 🙏

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🙇

@yanamura yanamura requested a review from ry-itto January 26, 2020 14:38
ry-itto
ry-itto approved these changes Jan 26, 2020
View reviewed changes
Copy link
Member

@ry-itto ry-itto left a comment

Choose a reason for hiding this comment

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

LGTM! 👍 I'll merge this.

@jmatsu-bot
Copy link
Collaborator

Your apk has been deployed to https://deploygate.com/distributions/e0b9127778d1cd842a5d94367bb165dfdf6478cb. Anyone can try your changes via the link.

Generated by 🚫 Danger

@jmatsu-bot
Copy link
Collaborator

No issue was reported. Cool!

Generated by 🚫 Danger

@ry-itto ry-itto merged commit aa01401 into DroidKaigi:master Jan 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ry-itto ry-itto ry-itto approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@yanamura @ry-itto @jmatsu-bot