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

Replace old placeholders & empty states #256

Merged
merged 31 commits into from
Apr 4, 2018
Merged

Replace old placeholders & empty states #256

merged 31 commits into from
Apr 4, 2018

Conversation

kvld
Copy link
Contributor

@kvld kvld commented Mar 22, 2018

Задача: #APPS-1817

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

Описание:
Для таблицы добавлено состояние с загрузкой.
Для контроллера добавлен протокол ControllerWithStepikPlaceholder (с дефолтной реализацией для UIViewController), который содержит методы registerPlaceholder(placeholder:for:) и showPlaceholder(placeholder:).

TODO

  • Сертификаты
  • Комментарии
  • Профиль
  • Модули
  • Уроки
  • Контроллер с карточками
  • Квиз
  • Уведомления
  • Выбор курса (адаптивный таргет)
  • Удалить неактуальные поды из проекта
    • DZNEmptyDataSet
    • PlaceholderView

@kvld
Copy link
Contributor Author

kvld commented Mar 23, 2018

@Ostrenkiy можно начинать тестить и смотреть код.

@kvld kvld changed the title [WIP] Replace old placeholders & empty states Replace old placeholders & empty states Mar 26, 2018
@@ -0,0 +1,106 @@
//
// ControllerWithStepikPlaceholder.swift
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.

StepikPlaceholderControllerContainer – вспомогательный класс. Думаю, что разделять его с протоколом не стоит.

}
}

internal var registeredPlaceholders: [PlaceholderState: StepikPlaceholder] = [:]
Copy link
Contributor

Choose a reason for hiding this comment

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

а почему ты постоянно используешь internal, а не private?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Другое дело, что эти поля можно явно не помечать как internal, потому что это уровень по умолчанию, тут я согласен.

@kvld kvld added this to the 1.56 milestone Apr 3, 2018
@kvld kvld merged commit 206a769 into dev Apr 4, 2018
@kvld kvld mentioned this pull request Apr 4, 2018
@kvld kvld deleted the feature/remove-dzn branch January 29, 2019 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants