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

Cards for adaptive courses #213

Merged
merged 37 commits into from
Jan 25, 2018
Merged

Cards for adaptive courses #213

merged 37 commits into from
Jan 25, 2018

Conversation

kvld
Copy link
Contributor

@kvld kvld commented Dec 26, 2017

Задача: #APPS-1696

Коротко для Release Notes, в формате «Сделали/Добавили/Исправили N»:
Добавили возможность проходить адаптивные курсы в режиме карточек

Описание:
В настройках добавлен переключатель, который включает отображение адаптивных курсов в виде стопки карточек с заданиями (нужно подумать о миграции).
В конфиге есть параметр adaptive.supportedCourses, который содержит массив айдишников курсов, для которых можем показывать такое отображение.
На виджете поддерживаемого курса вторая кнопка всегда "Инфо" (так как силлабус заменен на карточки).
Код из адаптивного приложения скопирован – код отрефакторен (в том числе и с поддержкой промисов), из него удалено всё лишнее. В будущем, если возникнет необходимость, код адаптивного приложения нужно наследовать (с поддержкой отдельных фич) от этого кода в основном приложении.

adaptive

TODO:

  • iPhone X
  • Локализация
  • Нативная анимация робота выпилен робот
  • Цвета
  • Текст поздравления
  • Аналитика
  • Онбординг
  • Синхронизация
  • Remote config для сервера с рейтингом и списка курсов
  • Повороты

@kvld kvld added the main label Dec 26, 2017
@kvld kvld self-assigned this Dec 26, 2017
@kvld kvld requested a review from Ostrenkiy December 26, 2017 14:52
@kvld
Copy link
Contributor Author

kvld commented Dec 28, 2017

@Ostrenkiy в этом PR просто добавляем карточки, чтобы это всё хорошо работало. Без игрофикации и прочего.

@Ostrenkiy
Copy link
Contributor

Ostrenkiy commented Jan 14, 2018

@kvld Еще нельзя смотреть этот PR?
Игрофикация все-таки сюда же будет добавляться?

@kvld
Copy link
Contributor Author

kvld commented Jan 16, 2018

@Ostrenkiy нет, смотри без игрофикации.
Здесь нужен ещё онбординг и remote config для поддержки списка адаптивных курсов.

@kvld kvld added this to the 1.51 milestone Jan 19, 2018
@Ostrenkiy
Copy link
Contributor

Подмерджь dev еще с новой версией свифта

Copy link
Contributor

@Ostrenkiy Ostrenkiy left a comment

Choose a reason for hiding this comment

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

Все хорошо. Как все реализовано будет, посмотрим как работает

private let overlayRightImageName = "overlay_simple"
private let overlayLeftImageName = "overlay_hard"

lazy var overlayImageView: UIImageView! = { [unowned self] in
Copy link
Contributor

Choose a reason for hiding this comment

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

Ух, опасные эти unowned
Здесь, конечно, это работает, но вообще почему не weak?
Чтобы лишний guard избежать?)
Меня просто немного смущает, что это все же замаскированный implicitly unwrapped optional.

Copy link
Contributor

Choose a reason for hiding this comment

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

Апдейт - там даже unowned не нужен
https://stackoverflow.com/questions/38141298/lazy-initialisation-and-retain-cycle

func scrollToQuizBottom()
}

class CardStepPresenter {
Copy link
Contributor

Choose a reason for hiding this comment

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

В будущем надо будет переиспользовать степы из основного приложения в адаптивном, в принципе примерно одно и то же делает код

Copy link
Contributor

Choose a reason for hiding this comment

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

Конечно, проще было бы, если бы все квизы переиспользовались (т.е. надо WebStepViewController подкрутить до адекватного состояния с возможностью переиспользования в адаптивном режиме и MVP)

Copy link
Contributor

Choose a reason for hiding this comment

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

Пока норм

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Я почти уверен, что мы будем вносить большое количество правок в код степов при их редизайне, тогда же предлагаю и вернуться к этому (карточки, по сути – это другое представление степов же).

@kvld kvld merged commit f036de3 into dev Jan 25, 2018
@Ostrenkiy Ostrenkiy mentioned this pull request Jan 28, 2018
@kvld kvld mentioned this pull request Feb 2, 2018
5 tasks
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.

2 participants