plasma-*: Fix Modal Overlay & Popup registration logic #1380
Conversation
vadim-kudr
requested review from
Yeti-or,
neretin-trike,
Yakutoc,
TitanKuzmich and
shuga2704
as code owners
|
Theme Builder app deployed! https://plasma.sberdevices.ru/pr/plasma-theme-builder-pr-1380/ |
|
Documentation preview deployed! website:https://plasma.sberdevices.ru/pr/pr-1380/ |
vadim-kudr
removed request for
Yeti-or,
Yakutoc,
neretin-trike,
TitanKuzmich and
denivladislav
⚡ Component performance testingResult: 🟢 OK |
vadim-kudr
changed the title
There was a problem hiding this comment.
if (index === -1) { return prevItems; }
Тут тоже нужно [...prevItems] ?
There was a problem hiding this comment.
не, ничего же не меняется, обновление контекста не нужно
There was a problem hiding this comment.
Получается в одном сеттере мы делаем и возврат без обновления и возврат с обновлением. Мне кажется, это неочевидным моментом в коде, который лучше обойти. Что думаешь?
Мб вернуться к варианту, когда мы сначала просто подбираем items из useState, ищем индекс, а дальше идет guard expression?
const index = items.findIndex((item: PopupInfo) => id === item.id);
if (index === -1) {
return;
}
There was a problem hiding this comment.
звучит хорошо. в текущей реализации еще useCallback не хватает. вот нужны ли они, изменение items все равно контекст обновит, нужно подумать
const register = useCallback(() => setState(() => ...), []);
const unregister = useCallback(() => setState(() => ...), []);
There was a problem hiding this comment.
что-то там с замыканиями было на items, поэтому через setItems сделано, ща попробую проще сделать
There was a problem hiding this comment.
в items старые значения, как-будто замыкание на что-то другое идет, поэтому через setItems(() => ...)
Yakutoc
changed the title
| @@ -65,7 +65,7 @@ export const modalRoot = (Root: RootProps<HTMLDivElement, ModalProps>) => | |||
| popupInfo, | |||
| }); | |||
|
|
|||
| const transparent = useMemo(() => getIdFirstModal(popupController.items) !== innerId, [ | |||
| const transparent = useMemo(() => getIdLastModal(popupController.items) !== innerId, [ | |||
There was a problem hiding this comment.
можно убрать, не стал старый код трогать
There was a problem hiding this comment.
можно убрать, не стал старый код трогать
Popup
popupsWhat/why changed
Сделали ошибку когда правили другой баг
фикс для PR #1325
📦 Published PR as canary version:
Canary Versions✨ Test out this PR locally via:
npm install @salutejs/plasma-asdk@0.133.1-canary.1380.10485553077.0 npm install @salutejs/plasma-b2c@1.375.1-canary.1380.10485553077.0 npm install @salutejs/plasma-new-hope@0.128.2-canary.1380.10485553077.0 npm install @salutejs/plasma-web@1.376.1-canary.1380.10485553077.0 npm install @salutejs/sdds-cs@0.105.1-canary.1380.10485553077.0 npm install @salutejs/sdds-dfa@0.103.1-canary.1380.10485553077.0 npm install @salutejs/sdds-serv@0.104.1-canary.1380.10485553077.0 # or yarn add @salutejs/plasma-asdk@0.133.1-canary.1380.10485553077.0 yarn add @salutejs/plasma-b2c@1.375.1-canary.1380.10485553077.0 yarn add @salutejs/plasma-new-hope@0.128.2-canary.1380.10485553077.0 yarn add @salutejs/plasma-web@1.376.1-canary.1380.10485553077.0 yarn add @salutejs/sdds-cs@0.105.1-canary.1380.10485553077.0 yarn add @salutejs/sdds-dfa@0.103.1-canary.1380.10485553077.0 yarn add @salutejs/sdds-serv@0.104.1-canary.1380.10485553077.0