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

feat(plasma-web/b2c/new-hope): Add TextArea component with new design and core #817

Merged
merged 11 commits into from
Nov 29, 2023

Conversation

neretin-trike
Copy link
Collaborator

@neretin-trike neretin-trike commented Oct 25, 2023

Release Notes

Добавлена реализация компонента TextArea в новой архитектуре.

Добавлены сборки компонента для новых тем plasma_web, plasma_b2c, sds_engineer с новыми пропсами: view, size, disabled.

Добавлены сборки компонента для старых библиотек @salutejs/plasma-web, @salutejs/plasma-b2c.

Добавлен механизм динамических лейблов, которые в зависимости от различных сценариев ведёт себя по-разному.

What/why Changed

Для plasma-web:

  • Исправлено поведение, при котором во время наведения на компонент в состоянии readonly при наличии статуса (success, error, warning) рамки красились в дефолтный цвет

Для plasma-b2c:

  • Исправлена компановка компонента, из-за которой все цвета были искажены
  • Исправлено изменение размера скруглений в зависимости от статуса
  • Исправлено наложение цветов для блока "хелперов" при наличии статуса
  • Исправлено отображение состояния disabled

Для plasma-b2c и plasma-web

  • Исправлена работа свойств width и height, которые не всегда отображали реальные размеры компонента
  • Исправлена работа свойств rows и cols, из-за которых количество строк / столбцов не всегда отображались корректно
  • Исправлена работа свойств minAuto и maxAuto, из-за которых минимальное и максимальное количество доступных строк не всегда отображались корректно

Остальное

Обновлены снапшоты тестов для библиотек plasma-web и plasma-b2c в соответствии со всеми исправлениями

Сохранены старые интерфейсы для библиотек plasma-web и plasma-b2c у компонента TextArea

Удалён компонент TextArea из @salutejs/plasma-hope (за исключением типов)

Обновлён workflow файл cypress-common. В scope добавлены зависимости plasma-tokens-b2c, т.к. в данном PR этот пакет затрагивается

⚡ Component performance testing

Result: 🟢 OK

Details

render:

Компонент Изменение Было Стало
packages/plasma-web/src/components/TextArea/TextArea.perftest.tsx#Default -1.64 pts {-(-37.38%)-} 4.38 pts 2.74 pts
packages/plasma-b2c/src/components/TextArea/TextArea.perftest.tsx#Default -1.59 pts {-(-36.90%)-} 4.32 pts 2.73 pts

⚡ Component performance testing

Result: 🟢 OK

Details

render:

Компонент Изменение Было Стало
packages/plasma-web/src/components/TextArea/TextArea.perftest.tsx#Default -0.90 pts {-(-26.85%)-} 3.33 pts 2.44 pts
📦 Published PR as canary version: Canary Versions

✨ Test out this PR locally via:

npm install @salutejs/[email protected]
npm install @salutejs/[email protected]
npm install @salutejs/[email protected]
npm install @salutejs/[email protected]
npm install @salutejs/[email protected]
npm install @salutejs/[email protected]
npm install @salutejs/[email protected]
npm install @salutejs/[email protected]
npm install @salutejs/[email protected]
npm install @salutejs/[email protected]
npm install @salutejs/[email protected]
npm install @salutejs/[email protected]
# or 
yarn add @salutejs/[email protected]
yarn add @salutejs/[email protected]
yarn add @salutejs/[email protected]
yarn add @salutejs/[email protected]
yarn add @salutejs/[email protected]
yarn add @salutejs/[email protected]
yarn add @salutejs/[email protected]
yarn add @salutejs/[email protected]
yarn add @salutejs/[email protected]
yarn add @salutejs/[email protected]
yarn add @salutejs/[email protected]
yarn add @salutejs/[email protected]


const outerRef = innerRef && 'current' in innerRef ? innerRef : createRef<HTMLTextAreaElement>();
const hasHelper = Boolean(leftHelper || rightHelper || helperText);
const newView = status !== undefined ? fallbackStatusMap[status] : view;

Check warning

Code scanning / Semgrep

Semgrep Finding: gitlab.eslint.detect-object-injection

Bracket object notation with user input is present, this might allow an attacker to access all properties of the object and even it's prototype, leading to possible code execution.
@Salute-Eva
Copy link
Contributor

Theme Builder app deployed!

http://plasma.sberdevices.ru/pr/plasma-theme-builder-pr-817/

@neretin-trike neretin-trike force-pushed the neretinaa/add-new-hope-textarea branch from aaf89a3 to 7b3f9f5 Compare October 25, 2023 08:31
@Salute-Eva
Copy link
Contributor

Theme Builder app deployed!

http://plasma.sberdevices.ru/pr/plasma-theme-builder-pr-817/

@@ -0,0 +1,149 @@
export const classes = {
/** Коммент */
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
Collaborator Author

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.

это мета шутка по Пелевину: коммент как бы есть, но его нет
ты его видишь но не осознаешь
и вообще если задуматься то коммент он в твоем сознании, а ты это коммент к своему сознанию, и если упростить и не задумываться то
коммент это ты, а ты это коммент
главное конeчно тут не разделять на главное и второстепенное
а то сразу ком в горле, холодок в ноздрях, а тут и мент придет
поэтому спокойней товарищ

Коммент конечен
как первый снег за окном
пора выходить

packages/plasma-new-hope/src/utils/index.ts Outdated Show resolved Hide resolved
} = classes;

export const applyDynamicLabel = `
.${String(innerPlaceholderUpClass)} {
Copy link
Contributor

Choose a reason for hiding this comment

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

уточнение
а зачем везде к String приводить?

Copy link
Collaborator Author

@neretin-trike neretin-trike Oct 27, 2023

Choose a reason for hiding this comment

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

иначе linaria при сборке ругается на строковые переменные у классов, т.к. они "могут быть undefined". Кароч бред, но без него билд не работает

Copy link
Collaborator

Choose a reason for hiding this comment

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

@Yeti-or Вот у Артем тоже использует приведение к строке. Это к твоему комментарию/вопросу к @TitanKuzmich

font-style: var(${tokens.fontStyle});
font-weight: var(${tokens.fontWeight});
letter-spacing: var(${tokens.letterSpacing});
line-height: var(${tokens.lineHeight});
Copy link
Contributor

Choose a reason for hiding this comment

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

вот думаю было б класнно как-то так:

const StyledLabel = styled.span`
  margin-bottom: var(${tokens.labelMarginBottom});

  ${...Typography.h4}
`

не ? или тут другая типографика?

или вообще как-то так:

styled(typo.h4)`
  margin-bottom: var(${tokens.labelMarginBottom});
`

или может

const Label = <H4 mb={var(${tokens.labelMarginBottom});} />

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ну вот теперь да, когда во многих местах появилось использование типографики кажется есть смысл вынести куда-нибудь. Но проблема в том, что там не просто Typography.h4, а токены типографики именно для конкретной части компонента. Типа TypographyHelpers.h4, TypographyInput.h4 и т.д.

`;

export const StyledLeftHelper = styled.span`
${applyEllipsis()};
Copy link
Contributor

Choose a reason for hiding this comment

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

@TitanKuzmich а в чем разница почему теме нужно было String() везде писать а @neretin-trike нет ?

@@ -0,0 +1,149 @@
export const classes = {
/** Коммент */
Copy link
Contributor

Choose a reason for hiding this comment

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

это мета шутка по Пелевину: коммент как бы есть, но его нет
ты его видишь но не осознаешь
и вообще если задуматься то коммент он в твоем сознании, а ты это коммент к своему сознанию, и если упростить и не задумываться то
коммент это ты, а ты это коммент
главное конeчно тут не разделять на главное и второстепенное
а то сразу ком в горле, холодок в ноздрях, а тут и мент придет
поэтому спокойней товарищ

Коммент конечен
как первый снег за окном
пора выходить

minAuto?: number;
}

export interface TextAreaPropsExtends extends TextAreaPropsBase {
Copy link
Collaborator

Choose a reason for hiding this comment

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

[Уточнение]

@neretin-trike
TextAreaPropsExtends нужен снаруже?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

не уверен, я наружу вообще не хотел типы экспортить, но тут хз

@neretin-trike neretin-trike force-pushed the neretinaa/add-new-hope-textarea branch 2 times, most recently from cb6305d to d0f5daa Compare November 2, 2023 15:58
@Salute-Eva
Copy link
Contributor

Theme Builder app deployed!

http://plasma.sberdevices.ru/pr/plasma-theme-builder-pr-817/

Copy link
Contributor

@Yeti-or Yeti-or left a comment

Choose a reason for hiding this comment

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

Я всё ещё раз посмотрел
меня конечно смущает что получилось тааак много токенов
и такие сложные селекторы
ещё смущает что мы не можем куда-то отдельно вытащить работу с resizeObserver в тот же вариант например
и не ясно в какой момент мы в plasma-web добавим размеры еще =/

Вообщем предлагаю вливать, единственный момент проверь пожалуйста состояния focus / hover у sds-enginer по макетам и надо сторис к новому формату привести

@Salute-Eva
Copy link
Contributor

Theme Builder app deployed!

http://plasma.sberdevices.ru/pr/plasma-theme-builder-pr-817/

Copy link
Contributor

⚡ Component performance testing

Result: 🟢 OK

Details

render:

Component Diff Base value Current value
packages/plasma-web/src/components/TextArea/TextArea.perftest.tsx#Default -3.24 pts (-53.04%) 6.12 pts 2.87 pts
packages/plasma-b2c/src/components/TextArea/TextArea.perftest.tsx#Default -3.19 pts (-52.01%) 6.13 pts 2.94 pts

@neretin-trike neretin-trike added this pull request to the merge queue Nov 29, 2023
Merged via the queue into dev with commit 9156346 Nov 29, 2023
26 checks passed
@neretin-trike neretin-trike deleted the neretinaa/add-new-hope-textarea branch November 29, 2023 14:36
@Salute-Eva
Copy link
Contributor

🚀 This PR is included in version: @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected] 🚀

3 similar comments
@Salute-Eva
Copy link
Contributor

🚀 This PR is included in version: @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected] 🚀

@Salute-Eva
Copy link
Contributor

🚀 This PR is included in version: @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected] 🚀

@Salute-Eva
Copy link
Contributor

🚀 This PR is included in version: @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected] 🚀

@Salute-Eva
Copy link
Contributor

🚀 This PR is included in version: @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected] 🚀

5 similar comments
@Salute-Eva
Copy link
Contributor

🚀 This PR is included in version: @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected] 🚀

@Salute-Eva
Copy link
Contributor

🚀 This PR is included in version: @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected] 🚀

@Salute-Eva
Copy link
Contributor

🚀 This PR is included in version: @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected] 🚀

@Salute-Eva
Copy link
Contributor

🚀 This PR is included in version: @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected] 🚀

@Salute-Eva
Copy link
Contributor

🚀 This PR is included in version: @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected] 🚀

@Salute-Eva
Copy link
Contributor

🚀 This PR is included in version: @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected], @salutejs/[email protected] 🚀

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.

5 participants