New profile architecture #299
Conversation
| super.awakeFromNib() | ||
| placeholderView.isSkeletonable = true | ||
|
|
||
| let margin = Double(arc4random()) / 0xFFFFFFFF * 20.0 |
There was a problem hiding this comment.
вот сейчас что-то странное с рандомом произошло, пояснишь?
There was a problem hiding this comment.
arc4random возвращает случайное число из [0; UInt32.max], делением просто получаем Double из [0; 1]
Лучше заменить 0xFFFFFFFF на UInt32.max, думаю.
| // Copyright © 2018 Alex Karpov. All rights reserved. | ||
| // | ||
|
|
||
| enum ProfileMenuBlock: RawRepresentable, Equatable { |
There was a problem hiding this comment.
А почему не сделать просто :String вместо RawRepresentable?
There was a problem hiding this comment.
Потому что enum со значением.
| } | ||
|
|
||
| class ProfilePresenter { | ||
| enum UserSeed { | ||
| case other(id: Int) | ||
| case `self`(id: Int) |
There was a problem hiding this comment.
ну слишком сильно ты любишь эту тему с ключевыми словами в кавычках)
может во избежание путаницы лучше сделать как-нибудь без этого?
There was a problem hiding this comment.
Не знаю, мне кажется здесь кавычки не особо мешают :)
| } | ||
|
|
||
| class ProfilePresenter { | ||
| enum UserSeed { |
There was a problem hiding this comment.
А какое предназначение у UserSeed? Почему мы не можем использовать просто User-а ?
There was a problem hiding this comment.
Обсудили это уже, чтобы удобнее поддерживать 3 случая.
| } | ||
|
|
||
| func showStreakTimeSelection(startHour: Int) { | ||
| // FIXME: strange picker injection. this vc logic should be in the presenter, i think |
There was a problem hiding this comment.
Пока нет роутинг слоя, мы не узнаем, как правильно))
There was a problem hiding this comment.
Если проще так - пусть будет так
| customPresentViewController(streakTimePickerPresentr, viewController: vc, animated: true, completion: nil) | ||
| } | ||
|
|
||
| func requestNotificationsPermissions() { |
There was a problem hiding this comment.
Проверь лишний раз плиз что с нотификациями ничего не сломалось. Понимаю, что это копипаст, но все равно страшновато
| var pinsMapContentView: PinsMapBlockContentView? | ||
|
|
||
| // Implementation of StreakNotificationsControlView in extension | ||
| var presenterNotifications: StreakNotificationsControlPresenter? |
There was a problem hiding this comment.
зачем тогда attach метод в экстеншне, если можно просто сетить это?
есть сомнения, нужен ли экстеншн, если у нас поле тут торчит (но это совсем мелочи и неважно, можешь как есть оставить)
There was a problem hiding this comment.
Метод как альтернатива полю в протоколе.
Экстеншен здесь для того, чтобы вынести в него логику StreakNotificationsControlView. В данной реализации у нас сам ProfileViewController является StreakNotificationsControlView (из-за меню с блоками) и хотелось всю реализацию спрятать отдельно.
Stepic/ProfileViewController.swift
Outdated
| } | ||
|
|
||
| var streaksTooltip: Tooltip? | ||
| @objc func settingsButtonPressed() { | ||
| // Bad route injection :( |
| func navigateToSettings() { | ||
| self.performSegue(withIdentifier: "showSettings", sender: nil) | ||
| } | ||
| private func buildInfoExpandableBlock() -> ContentExpandableMenuBlock? { |
There was a problem hiding this comment.
А почему ты решил для профиля все это перенести из презентера во view?
В любом случае, с Settings должно быть консистентно - там сейчас до сих пор в SettingsPresenter инициализируются блоки
There was a problem hiding this comment.
Да, Settings и CodeSettings тоже нужно так сделать.
| self.view = view | ||
| } | ||
|
|
||
| private var notificationTimeString: String? { |
There was a problem hiding this comment.
Надо разобраться будет со всеми подсчетами и форматированиями времени, мб в отдельный сервис их перенести. Сейчас это все хаотично раскидано по проекту.
|
Это вмердживаем для беты, остальные фиксы в отдельном PR-е |
Задача: #APPS-1883
Коротко для Release Notes, в формате «Сделали/Добавили/Исправили N»:
Добавили возможность открывать профили пользователей
Описание:
Весь код профиля переписан для более гибкого использования отдельных блоков (теперь каждый блок – модуль MVP).
TODO