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.

Sign up for GitHub

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

Jump to bottom

AppRoot и внешний portalTarget #5046

Closed
obergodmar opened this issue May 10, 2023 · 1 comment · Fixed by #5469
Closed

AppRoot и внешний portalTarget #5046

obergodmar opened this issue May 10, 2023 · 1 comment · Fixed by #5469
Assignees
@mendrew
Labels
cmp:app-root type:bug
Milestone
v5.6.1

Comments

@obergodmar
Copy link

Привет! В данный момент существует проблема с внешним кастомным root-элементом портала (проп portalRoot компонента AppRoot).

Если при инициализации передать свой элемент, то при анмаунте AppRoot этот элемент будет удален вот на этой строчке:

VKUI/packages/vkui/src/components/AppRoot/AppRoot.tsx

Line 77 in d3c22d7

portal?.parentElement?.removeChild(portal);

Кажется это странным, ведь проп был передан извне и должен являться readonly. Существует множество причин при которых приложение было перемонтировано, а удаление при этом какого либо элемента является ломающим фактором для всего остального функционала.
@inomdzhon inomdzhon added this to VKUI May 10, 2023
@github-project-automation github-project-automation bot moved this to 🗃 Backlog in VKUI May 10, 2023
@obergodmar
Copy link
Author

В качестве решения я бы предложил удалять созданный portalTarget, если он не был передан извне через проп. А в случае с пропсом не производить никакого клинапа.

@mendrew mendrew self-assigned this Jul 13, 2023
@mendrew mendrew moved this from 🗃 Backlog to 🔜 To do in VKUI Jul 13, 2023
mendrew added a commit that referenced this issue Jul 14, 2023
@mendrew
bug(AppRoot): don't remote passed portal root on unmount
7cd94b1 
fix: #5046
If portalRoot has been passed as a prop we shouldn't remove
it from the document on unmount.
@mendrew mendrew moved this from 🔜 To do to 🔧 In progress in VKUI Jul 14, 2023
@mendrew mendrew mentioned this issue Jul 14, 2023
Merged
@mendrew mendrew moved this from 🔧 In progress to 👀 In Review in VKUI Jul 14, 2023
@SevereCloud SevereCloud added this to the v5.6.2 milestone Jul 17, 2023
@inomdzhon inomdzhon modified the milestones: v5.6.2, v5.6.1 Jul 17, 2023
inomdzhon pushed a commit that referenced this issue Jul 17, 2023
@mendrew
bug(AppRoot): don't remote passed portal root on unmount (#5469)
9316c13 
fix: #5046
If portalRoot has been passed as a prop we shouldn't remove
it from the document on unmount.
@github-project-automation github-project-automation bot moved this from 👀 In Review to ✅ Done in VKUI Jul 17, 2023
vkcom-publisher pushed a commit that referenced this issue Jul 17, 2023
@actions-user
bug(AppRoot): don't remote passed portal root on unmount (#5469)
9321cfd 
fix: #5046
If portalRoot has been passed as a prop we shouldn't remove
it from the document on unmount.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mendrew mendrew

Labels
cmp:app-root type:bug
Projects
VKUI
Archived in project
Milestone
v5.6.1
Development

Successfully merging a pull request may close this issue.

bug(AppRoot): don't remove passed portal root on unmount VKCOM/VKUI
4 participants
@mendrew @inomdzhon @SevereCloud @obergodmar