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

fix(DatePicker, Popper, Popout): extend usePlacementChangeCallback logic #6906

Merged

Conversation

mendrew
Copy link
Contributor

@mendrew mendrew commented May 13, 2024


  • Unit-тесты

Описание

Исправляем поведение хука usePlacementChangeCallbace, который вызывает onPlacemenChange коллбэк при автоматическом изменении свойства placement.

Из-за того, что onPlacementChange не всегда вызывался когда это требовалось мы получали неправильно отрендеренные границы у CustomSelect, потому что CustomSelect мог считать что placement = "bottom" когда на самом деле дропдаун уже отрисовывался сверху.

Изменения

  • Исправляем вызов коллбэка при первом рендере.
    Для этого учитываем значение пропа placement переданного библиотеке floating-ui. Раньше мы его игнорировали и могло быть так, что при первом рендере CustomSelect ожидает, что placement="bottom", а floating-ui уже решил, что resolvedPlacement="top", но так как это первый рендер, то коллбэк не вызывается и внутреннее состояние CustomSelect не меняется, от того границы рисуются будто placement="bottom".

  • устанавливаем значение popupDirection у CustomSelect по умолчанию bottom и используем значение этого пропа как значение по умолчанию для внутреннего состояния popperPlacement, чтобы CustomSelect было известно куда будет рендерится по умолчанию dropdown. Он и так сейчас рендерится вниз, так как это значение по умолчанию у CustomSelectDropdown, но CustomSelect об этом не знает.

@github-actions github-actions bot added the patch Автоматизация: PR продублируется в ветку последнего минорного релиза для выпуска патча label May 13, 2024
Copy link
Contributor

github-actions bot commented May 13, 2024

size-limit report 📦

Path Size
JS 366.1 KB (+0.01% 🔺)
JS (gzip) 112.36 KB (+0.03% 🔺)
JS (brotli) 92.73 KB (+0.01% 🔺)
JS import Div (tree shaking) 1.42 KB (0%)
CSS 273.88 KB (0%)
CSS (gzip) 35.6 KB (0%)
CSS (brotli) 28.85 KB (0%)

Copy link

codesandbox-ci bot commented May 13, 2024

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

github-actions bot commented May 13, 2024

e2e tests

Playwright Report

Copy link
Contributor

github-actions bot commented May 13, 2024

👀 Docs deployed

Commit ad5ca26

Copy link

codecov bot commented May 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.32%. Comparing base (98d2b87) to head (ad5ca26).
Report is 13 commits behind head on master.

❗ Current head ad5ca26 differs from pull request most recent head 1dbad73. Consider uploading reports for the commit 1dbad73 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6906      +/-   ##
==========================================
- Coverage   83.33%   83.32%   -0.02%     
==========================================
  Files         350      350              
  Lines       10729    10452     -277     
  Branches     3575     3469     -106     
==========================================
- Hits         8941     8709     -232     
+ Misses       1788     1743      -45     
Flag Coverage Δ
unittests 83.32% <100.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mendrew mendrew changed the title fix(DatePicker): extend usePlacementChangeCallback logic fix(DatePicker, Popper, Popout): extend usePlacementChangeCallback logic May 16, 2024
@mendrew mendrew marked this pull request as ready for review May 16, 2024 09:39
@mendrew mendrew requested a review from a team as a code owner May 16, 2024 09:39
@mendrew mendrew merged commit 2549300 into master May 21, 2024
1 check passed
@mendrew mendrew deleted the mendrew/6889/fix/extended-popout-direction-calculation branch May 21, 2024 14:31
@vkcom-publisher
Copy link
Contributor

❌ Patch

Не удалось автоматически применить исправление на ветке 6.0-stable.

Дальнейшие действия выполняют контрибьютеры из группы @VKCOM/vkui-core

Чтобы изменение попало в ветку 6.0-stable, выполните следующие действия:

  1. Создайте новую ветку от 6.0-stable и примените изменения используя cherry-pick
git stash # опционально
git fetch origin 6.0-stable
git checkout -b patch/pr6906 origin/6.0-stable

git cherry-pick --no-commit 2549300b40f8664cf93657b12846383b6b4ae2e5
git checkout HEAD **/__image_snapshots__/*.png
git diff --quiet HEAD || git commit --no-verify --no-edit
  1. Исправьте конфликты, следуя инструкциям из терминала
  2. Отправьте ветку на GitHub и создайте новый PR с веткой 6.0-stable (установка лейбла не требуется!)
git push --set-upstream origin patch/pr6906
gh pr create --base 6.0-stable --title "patch: pr6906" --body "- patch #6906"

mendrew added a commit that referenced this pull request May 21, 2024
…gic (#6906)

Исправляем поведение хука `usePlacementChangeCallbace`, который вызывает `onPlacemenChange` коллбэк при автоматическом изменении свойства `placement`.

Из-за того, что `onPlacementChange` не всегда вызывался когда это требовалось мы получали неправильно отрендеренные границы у `CustomSelect`, потому что `CustomSelect` мог считать что `placement = "bottom"` когда на самом деле дропдаун уже отрисовывался сверху.

- Исправляем вызов коллбэка при первом рендере.
Для этого учитываем значение пропа `placement` переданного библиотеке `floating-ui`. Раньше мы его игнорировали и могло быть так, что при первом рендере `CustomSelect` ожидает, что `placement="bottom"`, а `floating-ui` уже решил, что `resolvedPlacement="top"`, но так как это первый рендер, то коллбэк не вызывается и внутреннее состояние `CustomSelect` не меняется, от того границы рисуются будто `placement="bottom"`.

- устанавливаем значение `popupDirection` у `CustomSelect` по умолчанию `bottom` и используем значение этого пропа как значение по умолчанию для внутреннего состояния `popperPlacement`, чтобы `CustomSelect` было известно куда будет рендерится по умолчанию dropdown. Он и так сейчас рендерится вниз, так как это значение по умолчанию у `CustomSelectDropdown`, но `CustomSelect` об этом не знает.
@mendrew mendrew mentioned this pull request May 21, 2024
@mendrew mendrew added this to the v6.1.0 milestone May 21, 2024
@inomdzhon inomdzhon removed the patch Автоматизация: PR продублируется в ветку последнего минорного релиза для выпуска патча label May 22, 2024
@vkcom-publisher
Copy link
Contributor

v6.1.0 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: DatePicker некорректные закругления при popupDirection="top"
4 participants