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

New application main color #112

Merged
merged 46 commits into from
Sep 15, 2017
Merged

New application main color #112

merged 46 commits into from
Sep 15, 2017

Conversation

Ostrenkiy
Copy link
Contributor

@Ostrenkiy Ostrenkiy commented Sep 12, 2017

Задача: #APPS-1418

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

Описание:

  • Добавлен StepikLabel со стилями, UILabel выпилен примерно везде.
  • Стили навигейшна теперь в StyledNavigationController.
  • Добавил в subview к navigationBar полупиксельную shadow линию, чтобы все хорошо работало на степах. Там в LessonViewController красота с использованием UINavigationControllerDelegate творится.
    update: Добавил аналогичное поведение и в FindCoursesViewController. Надо подумать, как этот код можно вынести для переиспользования, ибо сейчас просто копипаст.
  • UISearchBar после того, как поставили shadowView начал как-то неистово себя вести, постоянно расширяясь за экран - в FindCoursesViewController целых два костыля, которые фиксят поведение при нажатии на серчбар или появлении контроллера в UINavigation стеке (здесь пришлось прятать строку поиска и после transition-а быстро анимировать появление).
    update: Запилил кастомный серчбар так, что он не ломает все нафиг на iOS 11. А еще там анимашки прикольные.
  • Запилил много новых ассетов. Наконец, у нас появилась симпотная кнопка проигрывания видео и остальные ассеты в норм качестве (за исключением нескольких, которые не смог найти в SVG).
  • В целом, цвет navigation-а теперь можно менять на примерно любой. Нужно будет только перегенерить ассеты (но они все есть в скетч-проекте, поэтому не так больно).

new-colors-demo

@Ostrenkiy Ostrenkiy added the main label Sep 12, 2017
@Ostrenkiy Ostrenkiy added this to the 1.43 milestone Sep 12, 2017
@Ostrenkiy Ostrenkiy self-assigned this Sep 12, 2017
@Ostrenkiy Ostrenkiy requested a review from kvld September 12, 2017 22:16
@Ostrenkiy
Copy link
Contributor Author

Посмотрел, как интерфейс ведет себя на iOS 11. Выяснилось, что поиск в navigation bar-е просто ломает нам всё.
UISearchController подпилить так, чтобы он был не внутри UITableViewHeader-а я не смог, ибо он явно рассчитан на то, чтобы быть в header-е.
Просто UISearchBar подпилить тоже не вышло, потому что он какой-то забагованный, неанимируемый и некастомизируемый.
Написал свой CustomSearchBar, в который можно самим будет добавлять всякие плюшки и переиспользовать его дальше при редизайне (нужно только будет придумать абстракцию для контроллера а-ля UISearchController, только без баг, кастомизируемую и хорошо себя ведущую).

Также теперь точно хорошо работает навигация в ситуации, когда полоска тени на каком-то контроллере не нужна (смотреть extension с UINavigationControllerDelegate в классах FindCoursesViewController и LessonViewController). Нужно подумать, как переиспользовать код оттуда, ибо сейчас там просто копипаст.

Думаю, можно смотреть, тестировать и мерджить.

res += "\nh1{font-size: 20pt; font-family:Arial, Helvetica, sans-serif; line-height:1.6em; text-align: center;}"
res += "\nh2{font-size: 17pt; font-family:Arial, Helvetica, sans-serif; line-height:1.6em; text-align: center;}"
res += "\nh3{font-size: 14pt; font-family:Arial, Helvetica, sans-serif; line-height:1.6em; text-align: center;}"
res += "\nbody{font-size: 12pt; font-family:Arial, Helvetica, sans-serif; line-height:1.6em; color: #535366; }"
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.

В адаптивных мы разве тоже меняем цвет текста?

applyStyles()
}

/*
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.

Да, спасибо

static let navigationColor = stepicGreenColor()
static let mainLightColor: UIColor = UIColor(hex: 0xf6f6f6)
static let mainDarkColor: UIColor = UIColor(hex: 0x535366)
static let mainTextColor: UIColor = UIColor(hex: 0x535366)

class func backgroundColor() -> UIColor {
Copy link
Contributor

Choose a reason for hiding this comment

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

Может сразу отрефакторить это всё: сделать цвета свойствами, а не методами (как-то Swift 2 напоминает)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Да, давно пора)

@Ostrenkiy
Copy link
Contributor Author

Ostrenkiy commented Sep 14, 2017

Итого, надо

  • Все методы-цвета заменить на проперти
  • Выпилить комменты в новых файлах
  • Добавить @IBInspectable в StepikLabel и CustomSearchBar
  • Поаккуратнее посмотреть на стили в UIWebView
  • Поддержать кастомизацию всех цветов текста в адаптивных приложениях
  • Вынести в config-и все кастомные цвета

@Ostrenkiy
Copy link
Contributor Author

@kvld В адаптивном приложении для текста используются разные цвета в разных местах. Так что задачи про

  • Поддержать кастомизацию всех цветов текста в адаптивных приложениях
  • Вынести в config-и все кастомные цвета

остаются на тебе в следующий релиз. Ты потратишь сильно меньше времени на все это и сделаешь качественнее.

@Ostrenkiy Ostrenkiy merged commit d3b9bde into dev Sep 15, 2017
@Ostrenkiy Ostrenkiy deleted the feature/new-color branch September 15, 2017 17:11
Ostrenkiy added a commit that referenced this pull request Sep 15, 2017
@Ostrenkiy Ostrenkiy mentioned this pull request Sep 20, 2017
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