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

patch(v5): pr6567 #6571

Merged
merged 1 commit into from
Feb 14, 2024
Merged

patch(v5): pr6567 #6571

merged 1 commit into from
Feb 14, 2024

Conversation

mendrew
Copy link
Contributor

@mendrew mendrew commented Feb 14, 2024

…6567)

Убираем лишнюю оптимизацию из логики обновления позиции плавающего элемента.

У нас была дополнительная проверка на вызов коллбэка `MutationObserver`, с помощью которой мы пропускали обновление попапа при первом вызове коллбэка.
На сколько мы обсудили с @inomdzhon эта проверка была добавлена как оптимизация, чтобы не допустить лишнего ререндера при маунте.
Судя по результатам исследования у нас нету такой проблемы с ререндером из-за `MutationObserver` и эту проверку мы можем просто убрать.

А убрать нам её нужно, потому что она не даёт правильно спозиционировать опции `ChipsSelect`, если селект находится внизу окна, тем самым опции рендерятся сверху селекта. Если пользователь сразу после фокуса на селекте вводит текст, который оставляет одну-две опции в списке, то оставшиеся опции будут оторваны,  спозиционированы сильно выше селекта. Это происходит потому, что не срабатывает `update()` позиции опций, как раз из-за этой логики. И только при первом изменении размера списка опций. Если ввести текст, который вновь поменяет размер списка, позиционирование будет верным.
@mendrew mendrew added the patch Автоматизация: PR продублируется в ветку последнего минорного релиза для выпуска патча label Feb 14, 2024
@mendrew mendrew added this to the v5.10.1 milestone Feb 14, 2024
@mendrew mendrew self-assigned this Feb 14, 2024
Copy link
Contributor

github-actions bot commented Feb 14, 2024

size-limit report 📦

Path Size
JS 371.5 KB (-0.01% 🔽)
JS (gzip) 111.94 KB (-0.01% 🔽)
JS (brotli) 90.91 KB (+0.09% 🔺)
JS import Div (tree shaking) 2.74 KB (0%)
CSS 286.13 KB (0%)
CSS (gzip) 36.68 KB (0%)
CSS (brotli) 29.16 KB (0%)

Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Copy link
Contributor

e2e tests

Playwright Report

@mendrew mendrew marked this pull request as ready for review February 14, 2024 09:10
@mendrew mendrew requested a review from a team as a code owner February 14, 2024 09:10
Copy link
Contributor

👀 Docs deployed

Commit 5217ddf

@mendrew mendrew merged commit 520dfa0 into v5 Feb 14, 2024
45 checks passed
@mendrew mendrew deleted the mendrew/v5/patch-e8f5503 branch February 14, 2024 10:34
vkcom-publisher pushed a commit that referenced this pull request Feb 14, 2024
…6567) (#6571)

Убираем лишнюю оптимизацию из логики обновления позиции плавающего элемента.

У нас была дополнительная проверка на вызов коллбэка `MutationObserver`, с помощью которой мы пропускали обновление попапа при первом вызове коллбэка.
На сколько мы обсудили с @inomdzhon эта проверка была добавлена как оптимизация, чтобы не допустить лишнего ререндера при маунте.
Судя по результатам исследования у нас нету такой проблемы с ререндером из-за `MutationObserver` и эту проверку мы можем просто убрать.

А убрать нам её нужно, потому что она не даёт правильно спозиционировать опции `ChipsSelect`, если селект находится внизу окна, тем самым опции рендерятся сверху селекта. Если пользователь сразу после фокуса на селекте вводит текст, который оставляет одну-две опции в списке, то оставшиеся опции будут оторваны,  спозиционированы сильно выше селекта. Это происходит потому, что не срабатывает `update()` позиции опций, как раз из-за этой логики. И только при первом изменении размера списка опций. Если ввести текст, который вновь поменяет размер списка, позиционирование будет верным.
@inomdzhon inomdzhon removed this from the v5.10.1 milestone Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch Автоматизация: PR продублируется в ветку последнего минорного релиза для выпуска патча
Projects
Archived in project
3 participants