GIGADOC-380: Add ColorPicker#2709
Open
Olovyannikov wants to merge 2 commits into
Open
Conversation
Olovyannikov
requested review from
TitanKuzmich,
Yakutoc,
Yeti-or,
neretin-trike and
shuga2704
as code owners
Olovyannikov
force-pushed
the
gigadoc-380
branch
from
a300a72 to
16a2cc7
Compare
Contributor
|
Theme Builder app deployed! https://plasma.sberdevices.ru/pr/plasma-theme-builder-pr-2709/ |
Olovyannikov
force-pushed
the
gigadoc-380
branch
from
16a2cc7 to
6572433
Compare
Contributor
Olovyannikov
force-pushed
the
gigadoc-380
branch
from
6572433 to
4221fab
Compare
Olovyannikov
force-pushed
the
gigadoc-380
branch
from
de87d13 to
c980fca
Compare
Yakutoc
reviewed
Apr 20, 2026
Yakutoc
reviewed
Apr 20, 2026
Contributor
2026-04-20.13.59.53.mov
2026-04-20.14.11.25.mov2026-04-20.14.12.53.mov |
Olovyannikov
force-pushed
the
gigadoc-380
branch
from
ce0d5ed to
c44157d
Compare
Olovyannikov
force-pushed
the
gigadoc-380
branch
from
c44157d to
782630c
Compare
Collaborator
Author
|
Olovyannikov
force-pushed
the
gigadoc-380
branch
from
782630c to
1185882
Compare
shuga2704
requested changes
Apr 29, 2026
Contributor
shuga2704
left a comment
shuga2704
left a comment
There was a problem hiding this comment.
По мелочи оставил комментарии, а так очень хорошая большая работа 👍
Contributor
There was a problem hiding this comment.
А доки в plasma-giga-docs не будет для этого компонента?
Collaborator
Author
There was a problem hiding this comment.
Заведу, как только будет готов сам компонент с учетом правок
| value: number; | ||
| }) => React.ReactElement; | ||
| removeGradientStopSlot?: (props: { onRemove: () => void }) => React.ReactElement; | ||
| }; |
Contributor
There was a problem hiding this comment.
Кажется нужен extends от HTMLDivElement
Contributor
There was a problem hiding this comment.
И тогда кстати можно будет избавиться от className, style, width, height.
Contributor
There was a problem hiding this comment.
А, ну еще заомиттить onChange из HTMLDivElement, чтобы оно не смержилось с нашим пропсом
Collaborator
Author
There was a problem hiding this comment.
Низя. Тк во-первых потянется вериница dom-пропов, которые нам не нужны
Во-вторых поломается реф на forwarderRef
Contributor
There was a problem hiding this comment.
Низя. Тк во-первых потянется вериница dom-пропов, которые нам не нужны
принял, ок
Во-вторых поломается реф на forwarderRef
А почему он бы сломался? Ведь он у тебя тоже от HTMLDivElement
Collaborator
Author
There was a problem hiding this comment.
Ломается именно на уровне фабрики компонента, которая у вас вызывается, видимо из-за несогласованности типов или просто передаче целой кучи "мусорных" пропов из HtmlAttributes, которые вне не нужны
Contributor
There was a problem hiding this comment.
Так я слегка запутался.
- Я все равно не понял почему сломается типизация рефки, если типы заэкстендить к примеру от
React.HTMLAttributes<HTMLDivElement>. Рефка прекрасно будет работать также и без экстенда т.к. ее тип задается не в пропсах, а вforwardRef<HTMLDivElement, ColorPickerProps>.
Хотя мб я не так тебя понял изначально, раскрой мысль плз
- Вижу ты все таки добавил строку
} & HTMLAttributesWithoutOnChange<HTMLElement>;, хотя изначально все эти пропсы были не нужны. Столкнулся с какой-то проблемой?
Contributor
There was a problem hiding this comment.
А наш готовый Portal не сможешь переиспользовать?
Collaborator
Author
There was a problem hiding this comment.
А кинь как использовать и я посмотрю
Contributor
There was a problem hiding this comment.
собсна вот он packages/plasma-new-hope/src/components/Portal/index.ts, чекни если подойдет то вообще супер
| return () => window.removeEventListener('mouseup', handleUp); | ||
| }, []); | ||
|
|
||
| const handleColor = useCallback( |
Contributor
There was a problem hiding this comment.
точно нужен useCallback?
Collaborator
Author
There was a problem hiding this comment.
Да, мы не должны менять ссылку на функцию при изменении внешних факторов. React-way(
Contributor
There was a problem hiding this comment.
А я не вижу дальше нигде иной мемоизации, чтобы делать этот колбэк тоже мемным. Тогда зачем?
Collaborator
Author
There was a problem hiding this comment.
Убрал useCallback. dragPos менялся при каждом движении, и функция всё равно пересоздавалась.
Comment on lines
+27
to
+30
| console.log({ | ||
| selectedColor, | ||
| colors, | ||
| }); |
Contributor
There was a problem hiding this comment.
не забыть удалить в конце
Collaborator
Author
There was a problem hiding this comment.
Сделаю сейчас же
| import { Opacity } from './Opacity'; | ||
| import { Swatches } from './Swatches'; | ||
|
|
||
| export interface PickerProps { |
Contributor
There was a problem hiding this comment.
может лучше напикать все эти свойства из ColorPickerProps?
Olovyannikov
force-pushed
the
gigadoc-380
branch
from
1185882 to
ef92a4c
Compare
|
Important Review skippedToo many files! This PR contains 297 files, which is 147 over the limit of 150. To get a review, narrow the scope: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (297)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Core
ColorPicker
What/why changed
📦 Published PR as canary version:
Canary Versions✨ Test out this PR locally via:
npm install @salutejs/plasma-asdk@0.378.0-canary.2709.26571222811.0 npm install @salutejs/plasma-b2c@1.620.0-canary.2709.26571222811.0 npm install @salutejs/plasma-core@1.228.0-canary.2709.26571222811.0 npm install @salutejs/plasma-giga@0.347.0-canary.2709.26571222811.0 npm install @salutejs/plasma-homeds@0.347.0-canary.2709.26571222811.0 npm install @salutejs/plasma-hope@1.374.0-canary.2709.26571222811.0 npm install @salutejs/plasma-icons@1.239.0-canary.2709.26571222811.0 npm install @salutejs/plasma-new-hope@0.364.0-canary.2709.26571222811.0 npm install @salutejs/plasma-tokens-b2b@1.56.0-canary.2709.26571222811.0 npm install @salutejs/plasma-tokens-b2c@0.67.0-canary.2709.26571222811.0 npm install @salutejs/plasma-tokens-web@1.71.0-canary.2709.26571222811.0 npm install @salutejs/plasma-tokens@1.140.0-canary.2709.26571222811.0 npm install @salutejs/plasma-typo@0.44.0-canary.2709.26571222811.0 npm install @salutejs/plasma-ui@1.350.0-canary.2709.26571222811.0 npm install @salutejs/plasma-web@1.622.0-canary.2709.26571222811.0 npm install @salutejs/sdds-bizcom@0.352.0-canary.2709.26571222811.0 npm install @salutejs/sdds-cs@0.356.0-canary.2709.26571222811.0 npm install @salutejs/sdds-dfa@0.350.0-canary.2709.26571222811.0 npm install @salutejs/sdds-finai@0.343.0-canary.2709.26571222811.0 npm install @salutejs/sdds-insol@0.347.0-canary.2709.26571222811.0 npm install @salutejs/sdds-netology@0.351.0-canary.2709.26571222811.0 npm install @salutejs/sdds-os@0.22.0-canary.2709.26571222811.0 npm install @salutejs/sdds-platform-ai@0.351.0-canary.2709.26571222811.0 npm install @salutejs/sdds-sbcom@0.352.0-canary.2709.26571222811.0 npm install @salutejs/sdds-scan@0.350.0-canary.2709.26571222811.0 npm install @salutejs/sdds-serv@0.351.0-canary.2709.26571222811.0 npm install @salutejs/core-themes@0.31.0-canary.2709.26571222811.0 npm install @salutejs/plasma-themes@0.52.0-canary.2709.26571222811.0 npm install @salutejs/sdds-themes@0.67.0-canary.2709.26571222811.0 npm install @salutejs/sdds-api-tests@0.9.0-canary.2709.26571222811.0 npm install @salutejs/plasma-cy-utils@0.158.0-canary.2709.26571222811.0 npm install @salutejs/plasma-sb-utils@0.228.0-canary.2709.26571222811.0 npm install @salutejs/plasma-tokens-utils@0.52.0-canary.2709.26571222811.0 # or yarn add @salutejs/plasma-asdk@0.378.0-canary.2709.26571222811.0 yarn add @salutejs/plasma-b2c@1.620.0-canary.2709.26571222811.0 yarn add @salutejs/plasma-core@1.228.0-canary.2709.26571222811.0 yarn add @salutejs/plasma-giga@0.347.0-canary.2709.26571222811.0 yarn add @salutejs/plasma-homeds@0.347.0-canary.2709.26571222811.0 yarn add @salutejs/plasma-hope@1.374.0-canary.2709.26571222811.0 yarn add @salutejs/plasma-icons@1.239.0-canary.2709.26571222811.0 yarn add @salutejs/plasma-new-hope@0.364.0-canary.2709.26571222811.0 yarn add @salutejs/plasma-tokens-b2b@1.56.0-canary.2709.26571222811.0 yarn add @salutejs/plasma-tokens-b2c@0.67.0-canary.2709.26571222811.0 yarn add @salutejs/plasma-tokens-web@1.71.0-canary.2709.26571222811.0 yarn add @salutejs/plasma-tokens@1.140.0-canary.2709.26571222811.0 yarn add @salutejs/plasma-typo@0.44.0-canary.2709.26571222811.0 yarn add @salutejs/plasma-ui@1.350.0-canary.2709.26571222811.0 yarn add @salutejs/plasma-web@1.622.0-canary.2709.26571222811.0 yarn add @salutejs/sdds-bizcom@0.352.0-canary.2709.26571222811.0 yarn add @salutejs/sdds-cs@0.356.0-canary.2709.26571222811.0 yarn add @salutejs/sdds-dfa@0.350.0-canary.2709.26571222811.0 yarn add @salutejs/sdds-finai@0.343.0-canary.2709.26571222811.0 yarn add @salutejs/sdds-insol@0.347.0-canary.2709.26571222811.0 yarn add @salutejs/sdds-netology@0.351.0-canary.2709.26571222811.0 yarn add @salutejs/sdds-os@0.22.0-canary.2709.26571222811.0 yarn add @salutejs/sdds-platform-ai@0.351.0-canary.2709.26571222811.0 yarn add @salutejs/sdds-sbcom@0.352.0-canary.2709.26571222811.0 yarn add @salutejs/sdds-scan@0.350.0-canary.2709.26571222811.0 yarn add @salutejs/sdds-serv@0.351.0-canary.2709.26571222811.0 yarn add @salutejs/core-themes@0.31.0-canary.2709.26571222811.0 yarn add @salutejs/plasma-themes@0.52.0-canary.2709.26571222811.0 yarn add @salutejs/sdds-themes@0.67.0-canary.2709.26571222811.0 yarn add @salutejs/sdds-api-tests@0.9.0-canary.2709.26571222811.0 yarn add @salutejs/plasma-cy-utils@0.158.0-canary.2709.26571222811.0 yarn add @salutejs/plasma-sb-utils@0.228.0-canary.2709.26571222811.0 yarn add @salutejs/plasma-tokens-utils@0.52.0-canary.2709.26571222811.0