Skip to content

feat(cards): Added cards #48

Merged
merged 8 commits into from
Nov 1, 2018
Merged

Conversation

Dangel84
Copy link
Contributor

No description provided.

src/lib/cards/_cards-theme.scss Outdated Show resolved Hide resolved
commitlint.config.js Outdated Show resolved Hide resolved
src/lib/cards/cards.component.ts Outdated Show resolved Hide resolved
src/lib/cards/cards.component.ts Outdated Show resolved Hide resolved
src/lib/cards/cards.component.html Outdated Show resolved Hide resolved
src/lib/cards/cards.component.ts Outdated Show resolved Hide resolved
src/lib/cards/cards.md Outdated Show resolved Hide resolved
src/lib/cards/_cards-theme.scss Outdated Show resolved Hide resolved
@mikeozornin
Copy link
Contributor

Компоненты
Хувер не совпадает с макетом. Но если в коде какой-то общий хувер, то макет можно подогнать.

Рамка фокуса 1пк, а должна быть 2пк. Второй пиксель цеплин может не показывать бордером, там тень. Это сделано, чтобы физические размеры карточки не увеличивались, если рамка утолщается.

В дефолтном состоянии левая цветая полоска перекрывается границей: http://d.mikeozornin.ru/Ry4c1b
В состоянии селекта все ок.

$info: map-get($theme, primary);
Инфо и праймари — разные цвета. Конкретно у нас они совпадают (возможно даже пока), но вообще разные. Инфо всегда синенький, а праймари может быть другого цвета, например красный или желтый.

$error: map-get($theme, warn);
$warning: $mc-yellow;
$success: $mc-green;
Переменная error называется warn, а для других переменных вообще нет.
@fost, загляни сюда.

У неинтерактивных можно сменить состояние: http://d.mikeozornin.ru/vbFvU5
Если это из-за примера, то наверное можно не чинить.

Карточка в фокусе не выбирается с клавиатуры. Нужно реакцию на пробел.

Доки
В доках много стилей не из мозаики самой, будет сложно мигрировать доки в темную тему: текст не того кегля и цвета, иконки не тех цветов.

Опечатка в «Неинетерактивные»

Copy link
Contributor

@mikeozornin mikeozornin left a comment

Choose a reason for hiding this comment

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

@pimenovoleg
Copy link
Member

Надо обратить внимание на макет.
В нем контент не является частью компонента.

# Conflicts:
#	src/lib/core/theming/_all-theme.scss
#	src/lib/core/theming/_palette.scss
#	src/lib/core/theming/prebuilt/dark-theme.scss
@lskramarov
Copy link
Contributor

PR подвис... что осталось и в чем сложности ?

@mikeozornin
Copy link
Contributor

  • По неведомым причинам фокус в в светлой теме в макете не primary-500, а primary-400.
    @lskramarov, мне кажется для дефолтного праймари цвета (у нас 500) надо использовать уже какую-то переменную, иначе из-за косяков в макетах будем путаться.
    Возможно эти переменные уже есть, в pre-build-темах есть такое:
    $primary: mc-palette($mc-blue, 500, 600, 600);
    Возможно первое число — то, что нам как раз нужно.
@if $isDark == true {
        @include set-state(mc-color($color, 700), mc-color($color, 600), mc-color($border-color, 600));
    } @else {
        @include set-state(mc-color($color, 60), mc-color($color, 400), mc-color($border-color, 100));
    }

@lskramarov говорит, что isDark не используем. Видимо, нужно вынести переменные в pallette.
Кажется, понадобится:

  • по 1 переменной на каждый семантический фон (те, что в светлой теме -60)

вот эти цвета нужны не только карточкам:

  • white-фон карточки будет взять из общей палитры
  • цвет оверлея для выбранности (он все равно понадобится много где ещё)

А эти можно попробовать изменить так, чтобы переменные не понадобились:

  • левую полоску можно выбрать цвета одинаково хорошо видного в обеих темах (попробовать взять -500).

Мне:

  • починить макеты
  • починить в темной теме выбранное состояние, сейчас его не видно

# Conflicts:
#	src/lib/core/theming/_palette.scss
@lskramarov
Copy link
Contributor

@lskramarov, мне кажется для дефолтного праймари цвета (у нас 500) надо использовать уже какую-то переменную, иначе из-за косяков в макетах будем путаться.

Что бы использовать default заданный в: "$primary: mc-palette($mc-blue, 500, 600, 600);" нужно делать так: "mc-color($primary)" т.е. без параметров hue, берется дефолтный цвет, который мы установили ранее.

@lskramarov говорит, что isDark не используем. Видимо, нужно вынести переменные в pallette.

Давай не будем требовать автора делать этот компонент сразу в тёмной теме ? Поскольку у нас пока нет достаточного количества компонентов в темной теме. Я думаю на этой неделе сделаем еще несколько и тогда можно будет ссылаться на них и требовать такой же реализации.

src/lib/card/card.component.ts Outdated Show resolved Hide resolved
src/lib/card/_card-theme.scss Outdated Show resolved Hide resolved
src/lib/card/_card-theme.scss Outdated Show resolved Hide resolved
src/lib/card/_card-theme.scss Outdated Show resolved Hide resolved
@Dangel84
Copy link
Contributor Author

Dangel84 commented Oct 29, 2018

@lskramarov, мне кажется для дефолтного праймари цвета (у нас 500) надо использовать уже какую-то переменную, иначе из-за косяков в макетах будем путаться.

Что бы использовать default заданный в: "$primary: mc-palette($mc-blue, 500, 600, 600);" нужно делать так: "mc-color($primary)" т.е. без параметров hue, берется дефолтный цвет, который мы установили ранее.

На всякий случай уточню, что судя по коду, чтобы получить primary 500, надо написать "mc-color($primary, lighter)". Сделал так у себя. Но да, хорошо бы это отразить в макетах.

@lskramarov говорит, что isDark не используем. Видимо, нужно вынести переменные в pallette.

Давай не будем требовать автора делать этот компонент сразу в тёмной теме ? Поскольку у нас пока нет достаточного количества компонентов в темной теме. Я думаю на этой неделе сделаем еще несколько и тогда можно будет ссылаться на них и требовать такой же реализации.

Дело в том, что от "автора" реализации в темной теме требует проект. Есть предложение, если сильных возражений нету залить текущую реализацию, а потом сделать еще уточняющую итерацию, когда двсе детали по реализации темной темы будут утрясены.

@lskramarov
Copy link
Contributor

Да, согласен влить стили для темной темы как есть.
Остальное нужно поправить.

@lskramarov
Copy link
Contributor

  • нужно поправить упавшие тесты

image

@mikeozornin
Copy link
Contributor

Давай не будем требовать автора делать этот компонент сразу в тёмной теме ?

Да, конечно, давай.

Тогда нужно заменить цвет фокуса на -500 (в макеты залью апдейт на днях) и можно вливать.

Косяки макетов поправим когда будем адаптировать к темной теме.

@pimenovoleg pimenovoleg merged commit dffeeec into positive-js:master Nov 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants