[FRNT-532, FRNT-532] Implement box model, rewrite ButtonIcon and Chip #109

tatinacher · 2021-05-06T09:44:32Z

Implement box model, rewrite ButtonIcon and Chip

[FRNT-435, FRNT-532]

Irinaristova · 2021-05-06T13:27:35Z

src/ui/atoms/button-icon/index.tsx

-  --local-padding: calc(1px * var(--woly-component-level) * var(--woly-main-level));
+  --local-icon-size: calc(var(--woly-line-height));
+  --local-button-size: calc(var(--local-icon-size) + (var(--local-vertical) * 2));
+  --local-icon-color: var(--woly-shape-default);


на макете цвета иконок черные и в primary, и в secondary

цвета не меняла, ток у Chip чуть поправила чтоб его видно было и удобно было смотреть, но пока не как в макетах, задачи такой не стояло

а есть смысл заводить задачу только на то, чтобы 1 переменную для цвета поменять?

по идее надо две задачи, для чипа проверить все цвета для вариантов и для иконок. плюс из такого что щас вижу - у иконок еще появилась подложка с прозрачностью.

ты просто в этой таске уже начала рефакторить цвета иконки, тут тогда не надо было совсем этого делать

Irinaristova · 2021-05-06T13:34:53Z

src/ui/atoms/chip/index.tsx


 interface ChipProps {
-  action?: React.ReactNode;
  children?: string;


при клике на иконку у chip состояние active отображается всегда только для блока, iconButton никак не меняется

в этой ветке смотрим box-model, состояния как в макетах я не делала

Irinaristova · 2021-05-06T13:37:57Z

src/ui/atoms/chip/index.tsx


 interface ChipProps {
-  action?: React.ReactNode;
  children?: string;
  className?: string;


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

ага, надо поправить, но в отдельной задаче по правке состояний чипа

Irinaristova · 2021-05-06T13:44:47Z

src/ui/atoms/chip/index.tsx


 interface ChipProps {
-  action?: React.ReactNode;
  children?: string;
  className?: string;
  disabled?: boolean;


иконка залезает на слова

да, для нулевого размера пока не знаю как cделать, в формулах gap = 0 и изза этого outline который выходит за ButtonIcon перекрывает текст

тут нет outline у icon, это подсвечивается сам chip, icon-button никак не меняется

тут дело не в icon-button, а в формуле компенсации для текста, у тебя текст может быть любой длины, при этом, иконка не должна залезать на него. Плюс у тебя сама иконка меньше по размерам, чем высота chip, а так не должно быть, по высоте они должны быть одинаковыми

а я тут вижу что иконка в фокусе 👀

Тут надо обсудить уже подробнее и мб с дизайнерами, потому что пока что реализовано, что если фокус на иконку то вся плашка чипа тоже в фокусе, и вроде с Димой Марисоным обсуждали это и он говорил что это ок.

можем втроем поглядеть, чтобы никому обидно не было ))

Irinaristova · 2021-05-06T13:52:14Z

src/ui/atoms/chip/index.tsx

  children?: string;
  className?: string;
  disabled?: boolean;
-  icon?: React.ReactNode;


у тебя chip размера xs, то есть он должен быть по размеру как иконка === 24px, и в примере, где есть иконка она должна быть вплотную с границами chip, а вылезать за пределы должна только box-shadow => т.е. нужно вычесть ширину border, иначе он не будет соответствовать размерам icon-button и не будет вмещаться в input, к примеру

для XS размер 30, это у размера N - 24px.
бордеры учта и правила, погляди точно свежая версия ветки

а, ок, думала, что размер xs самый мелкий. Но вот размер N и по размерам он равен 24px, ветку подтянула.

Irinaristova · 2021-05-06T13:53:32Z

src/ui/atoms/chip/index.tsx

  children?: string;
  className?: string;
  disabled?: boolean;
-  icon?: React.ReactNode;
-  onActionClick?: (event: React.MouseEvent<HTMLButtonElement>) => void;


вот скрин с макета

в чем вопрос?

это скрин к комменту выше

Irinaristova · 2021-05-07T09:54:14Z

src/ui/atoms/chip/index.tsx

  children?: string;
  className?: string;
  disabled?: boolean;
-  icon?: React.ReactNode;
-  onActionClick?: (event: React.MouseEvent<HTMLButtonElement>) => void;
-  onClick?: (event: React.MouseEvent<HTMLDivElement>) => void;


chip зазмера s у тебя по высоте 36px, а должен быть 30 px по макету: высота шрифта и иконки 24px + 6px - отступ

В нашей модели N = 24, XS = 30, S = 36, и тп

ок ) выше написала коммент )

sergeysova · 2021-05-07T11:48:18Z

src/ui/atoms/chip/index.tsx

  children,
  className,
  disabled,
-  icon,
-  onActionClick,
+  leftIcon,


Зачем снова переименовывать иконки?

Оно так называлось, потому что иконки имели разную принадлежность. Это не просто иконка слева и справа. Это иконка чипа и иконка действия. Если вдруг добавим rtl получится криво, что leftIcon внезапно начнет отображаться справа.

Мне кажется, лучше переименовать обратно, или в chipIcon, actionIcon

sergeysova · 2021-05-07T11:49:20Z

src/lib/box-styles.tsx

@@ -6,6 +6,11 @@ export const Global = styled.div`
    font-family: 'Helvetica Neue', sans-serif;
  }

+  button {


Тот же комментарий что и в https://github.com/woly-ui/woly/pull/110/files#r628142059

sergeysova · 2021-05-07T11:50:11Z

src/static/icons/close.svg

@@ -0,0 +1,3 @@
+<svg width="24" height="24" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg">


Почему close? Если это крестик, то ни для чего другого его нельзя использовать? Например, для удаления.

sergeysova · 2021-05-07T11:51:16Z

src/ui/atoms/button-icon/index.tsx

@@ -22,24 +23,38 @@ const ButtonIconBase: React.FC<Props & Variant> = ({
 );

 export const ButtonIcon = styled(ButtonIconBase)`
+  ${box}
+  --local-vertical: calc(


Может унести local-vertical внутрь box?

sergeysova · 2021-05-07T11:51:29Z

src/ui/atoms/button-icon/index.tsx

-  --local-icon-size: var(--woly-line-height);
-  --local-icon-color: var(--woly-canvas-text-default);
-  --local-padding: calc(1px * var(--woly-component-level) * var(--woly-main-level));
+  --local-icon-size: calc(var(--woly-line-height));


а зачем тут calc?

sergeysova · 2021-05-07T11:52:53Z

src/ui/atoms/chip/index.tsx

      </div>
+      {rightIcon && <div data-icon>{rightIcon}</div>}


Иконка справа это действие с чипом, обычно закрытие.
И по умолчанию иконка действия должна быть передана — крестик.

tatinacher added the @atoms Changes inside atoms label May 6, 2021

tatinacher requested review from sergeysova and Irinaristova May 6, 2021 09:44

tatinacher force-pushed the feat/FRNT-532-implement-box-model branch from e5af3fe to 66f5b5e Compare May 6, 2021 09:45

tatinacher marked this pull request as draft May 6, 2021 10:54

tatinacher added 2 commits May 6, 2021 14:40

feat: implement box-model and rewrite ButtonIcon

758056d

fix: rewrite Chip on box-model

857c804

tatinacher force-pushed the feat/FRNT-532-implement-box-model branch from 66f5b5e to 857c804 Compare May 6, 2021 11:41

tatinacher marked this pull request as ready for review May 6, 2021 11:41

Irinaristova reviewed May 6, 2021

View reviewed changes

Irinaristova reviewed May 7, 2021

View reviewed changes

sergeysova requested changes May 7, 2021

View reviewed changes

sergeysova merged commit 7adcecc into master May 7, 2021

sergeysova deleted the feat/FRNT-532-implement-box-model branch May 7, 2021 12:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FRNT-532, FRNT-532] Implement box model, rewrite ButtonIcon and Chip #109

[FRNT-532, FRNT-532] Implement box model, rewrite ButtonIcon and Chip #109

tatinacher commented May 6, 2021

Irinaristova May 6, 2021

tatinacher May 7, 2021

Irinaristova May 7, 2021

tatinacher May 7, 2021

Irinaristova May 7, 2021

Irinaristova May 6, 2021

tatinacher May 7, 2021

Irinaristova May 6, 2021

tatinacher May 7, 2021

Irinaristova May 6, 2021

tatinacher May 7, 2021

Irinaristova May 7, 2021

Irinaristova May 7, 2021

tatinacher May 7, 2021

tatinacher May 7, 2021 •

edited

Loading

Irinaristova May 7, 2021

Irinaristova May 6, 2021

tatinacher May 7, 2021

Irinaristova May 7, 2021

Irinaristova May 6, 2021

tatinacher May 7, 2021

Irinaristova May 7, 2021

Irinaristova May 7, 2021

tatinacher May 7, 2021

Irinaristova May 7, 2021

sergeysova May 7, 2021

sergeysova May 7, 2021

sergeysova May 7, 2021

sergeysova May 7, 2021

sergeysova May 7, 2021

sergeysova May 7, 2021

		@@ -0,0 +1,3 @@
		<svg width="24" height="24" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg">

[FRNT-532, FRNT-532] Implement box model, rewrite ButtonIcon and Chip #109

[FRNT-532, FRNT-532] Implement box model, rewrite ButtonIcon and Chip #109

Conversation

tatinacher commented May 6, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tatinacher May 7, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tatinacher May 7, 2021 •

edited

Loading