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

refactor(floating): mv all floating components to same API #6212

Merged
merged 3 commits into from
Dec 6, 2023

Conversation

inomdzhon
Copy link
Contributor

@inomdzhon inomdzhon commented Dec 5, 2023


  • Документация фичи
  • Гайд миграции

    в теле коммита

Note

Юниты и e2e будут позже отдельным PR.

Описание

  • Приводим API к единому виду.
  • Tooltip переименовываем OnboardingTooltip.
  • TextTooltip делаем стабильным и переименовываем в Tooltip. Используем стили от TooltipBase.
Подробности в migration_v6.md

Tooltip -> OnboardingTooltip

<OnboardingTooltip
- isShown
+ isShown

- alignX="bottom"
- alignY="left"
+ placement="bottom-start"

- offsetX={0}
+ offsetByCrossAxis={0}

- offsetY={0}
+ offsetByMainAxis={0}

- cornerOffset={0}
- cornerAbsoluteOffset={0}
+ arrowCornerOffset={0}
+ arrowCornerAbsoluteOffset={0}
>
  <div>Target</div>
</OnboardingTooltip>

TextTooltip -> Tooltip

<Tooltip
- autoUpdateOnTargetResize
- customMiddlewares
- renderContent
- getRef

- offsetSkidding={0}
+ offsetByCrossAxis={0}

- offsetDistance={0}
+ offsetByMainAxis={0}

- shownDelay={0}
- hideDelay={10}
+ hoverDelay={[0, 10]}

- forcePortal
+ usePortal

- portalRoot={someHTMLElement}
+ usePortal={someHTMLElement}
>
  <div>Target</div>
</Tooltip>

[!NOTE]
Поправить в migration_v6.md

Popper

<Popper
- renderContent

- arrowClassName=""
+ arrowProps={{ iconClassName: "" }}

- offsetDistance={0}
+ offsetByMainAxis={0}

- offsetSkidding={0}
+ offsetByCrossAxis={0}

- onPlacementChange={({ placement }) => {}}
+ onPlacementChange={(placement) => {}}

- forcePortal
+ usePortal

- portalRoot={someHTMLElement}
+ usePortal={someHTMLElement}
/>
  <div>Target</div>
</Popper>
  • onPlacementChange теперь вызывается только в случае, если Popper подобрал оптимальный placement вместо пользовательского.
  • renderContent удалён в пользу children. Раньше из renderContent можно было получить className, который задаёт Popper, сейчас этот className пустой.
  • targetRef теперь умеет принимать VirtualElement.
  • arrowProps принимает атрибуты HTMLDivElement, а также iconStyle и iconClassName.

Изменения

  • Для Popper, Popover и Tooltip используем AppRootPortal.
  • lib/floating/types – теперь это папка types/ с файлами, на данный момент, common.ts и component.ts.
  • В lib/floating/types/component выносим общие св-ва и их описание. В Popper, Popover, Tooltip и OnboardingTooltip выбираем самое необходимое.
  • TootlipBase
    • withArrow, arrowCoords, arrowPlacement, getArrowRef, getRootRef – объединил в arrowProps.
    • floatingStyle – используем style. В рамках этого изменения, удалил обёртку, на которую вешался floatingStyle.
      review: fix arrow offset property
  • Переименовал PopperArrow на FloatingArrow. Расширил API с offset и isStaticOffset.
  • OnboardingTooltip
    • Заменил UseFloatingMiddleware для смещения стрелки на параметры в FloatingArrow.
    • Заменил arrowCornerOffset на arrowOffset.
    • Заменил arrowCornerAbsoluteOffset на isStaticArrowOffset.

@inomdzhon inomdzhon requested a review from a team as a code owner December 5, 2023 12:23
Copy link
Contributor

github-actions bot commented Dec 5, 2023

size-limit report 📦

Path Size
JS 348.06 KB (+0.13% 🔺)
JS (gzip) 106.29 KB (+0.08% 🔺)
JS (brotli) 87.76 KB (+0.09% 🔺)
JS import Div (tree shaking) 1.43 KB (0%)
CSS 258.5 KB (-0.54% 🔽)
CSS (gzip) 33.79 KB (-0.43% 🔽)
CSS (brotli) 27.42 KB (-0.22% 🔽)

Copy link

codesandbox-ci bot commented Dec 5, 2023

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.

Latest deployment of this branch, based on commit 42744a7:

Sandbox Source
VKUI TypeScript Configuration

Copy link
Contributor

github-actions bot commented Dec 5, 2023

e2e tests

Playwright Report

Copy link
Contributor

github-actions bot commented Dec 5, 2023

👀 Docs deployed

Commit 42744a7

Copy link

codecov bot commented Dec 5, 2023

Codecov Report

Attention: 21 lines in your changes are missing coverage. Please review.

Comparison is base (59f83cd) 80.68% compared to head (42744a7) 81.07%.

Files Patch % Lines
...components/OnboardingTooltip/OnboardingTooltip.tsx 84.74% 9 Missing ⚠️
...ages/vkui/src/components/AppRoot/AppRootPortal.tsx 76.47% 4 Missing ⚠️
packages/vkui/src/components/Tooltip/Tooltip.tsx 92.00% 4 Missing ⚠️
...kui/src/components/FloatingArrow/FloatingArrow.tsx 83.33% 2 Missing ⚠️
...ingWithInteractions/useFloatingWithInteractions.ts 84.61% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6212      +/-   ##
==========================================
+ Coverage   80.68%   81.07%   +0.38%     
==========================================
  Files         325      324       -1     
  Lines       10103    10090      -13     
  Branches     3398     3385      -13     
==========================================
+ Hits         8152     8180      +28     
+ Misses       1951     1910      -41     
Flag Coverage Δ
unittests 81.07% <88.94%> (+0.38%) ⬆️

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.

@inomdzhon inomdzhon force-pushed the imirdzhamolov/issue2769/refactor-tooltips branch from bfe42ed to 7120ffb Compare December 5, 2023 12:45
SevereCloud
SevereCloud previously approved these changes Dec 5, 2023
mendrew
mendrew previously approved these changes Dec 6, 2023
Copy link
Contributor

@mendrew mendrew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

SevereCloud
SevereCloud previously approved these changes Dec 6, 2023
BlackySoul
BlackySoul previously approved these changes Dec 6, 2023
h2. ~~`Tooltip`~~ -> [OnboardingTooltip](#/OnboardingTooltip)

```diff
<OnboardingTooltip
- isShown
+ isShown

- alignX="bottom"
- alignY="left"
+ placement="bottom-start"

- offsetX={0}
+ offsetByCrossAxis={0}

- offsetY={0}
+ offsetByMainAxis={0}

- cornerOffset={0}
- cornerAbsoluteOffset={0}
+ arrowCornerOffset={0}
+ arrowCornerAbsoluteOffset={0}
>
  <div>Target</div>
</OnboardingTooltip>
```

h2. ~~`TextTooltip`~~ -> [Tooltip](#/Tooltip)

```diff
<Tooltip
- autoUpdateOnTargetResize
- customMiddlewares
- renderContent
- getRef

- offsetSkidding={0}
+ offsetByCrossAxis={0}

- offsetDistance={0}
+ offsetByMainAxis={0}

- shownDelay={0}
- hideDelay={10}
+ hoverDelay={[0, 10]}

- forcePortal
+ usePortal

- portalRoot={someHTMLElement}
+ usePortal={someHTMLElement}
>
  <div>Target</div>
</Tooltip>
```

h1. ⚠️ Поправить в migration_v6.md

h2. [Popper](#/Popper)

```diff
<Popper
- renderContent

- arrowClassName=""
+ arrowProps={{ iconClassName: "" }}

- offsetDistance={0}
+ offsetByMainAxis={0}

- offsetSkidding={0}
+ offsetByCrossAxis={0}

- onPlacementChange={({ placement }) => {}}
+ onPlacementChange={(placement) => {}}

- forcePortal
+ usePortal

- portalRoot={someHTMLElement}
+ usePortal={someHTMLElement}
/>
  <div>Target</div>
</Popper>
```

- `onPlacementChange` теперь вызывается только в случае, если `Popper` подобрал оптимальный
  `placement` вместо пользовательского.
- `renderContent` удалён в пользу `children`. Раньше из `renderContent` можно было получить
  `className`, который задаёт `Popper`, сейчас этот `className` пустой.
- `targetRef` теперь умеет принимать `VirtualElement`.
- `arrowProps` принимает атрибуты `HTMLDivElement`, а также `iconStyle` и `iconClassName`.
- Заменил `arrowCornerOffset` на `arrowOffset`.
- Заменил `arrowCornerAbsoluteOffset` на `isStaticArrowOffset`.
- Переименовал PopperArrow на FloatingArrow
- Заменил UseFloatingMiddleware для смещения стрелки на параметр
   в FloatingArrow.
@inomdzhon inomdzhon force-pushed the imirdzhamolov/issue2769/refactor-tooltips branch from e06d78e to 42744a7 Compare December 6, 2023 12:39
@inomdzhon inomdzhon merged commit c7e4c41 into master Dec 6, 2023
25 checks passed
@inomdzhon inomdzhon deleted the imirdzhamolov/issue2769/refactor-tooltips branch December 6, 2023 12:58
@inomdzhon inomdzhon mentioned this pull request Jan 29, 2024
1 task
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.

[Feature] Привести API всплывающих элементов к единому виду
4 participants