BREAKING CHANGE: Encapsulate Popper API by ling1726 · Pull Request #22586 · microsoft/fluentui

ling1726 · 2022-04-21T15:27:51Z

⚠️ BREAKING CHANGES

Offset array -> Offset object

Use the same API as Floating UI

Before

<Popover positioning={{ offset: [0, 10] }} />

After

<>
  <Popover positioning={{ offset: { crossAxis: 0, mainAxis: 10 } }} />
  {/* main axis shorthand */}
  <Popover positioning={{ offset: 10 }} />
</>

Offset function argument

Stops using third party library naming and concepts in the offset function. Instead of using Popper placement, uses position and align concepts from the library:

Before

export type OffsetFunctionParam = {
  popper: Rect;
  reference: Rect;
  placement: Placement;
};

After

export type OffsetFunctionParam = {
  positionedElement: Rect;
  target: Rect;
  position: Position;
  alignment: Alignment;
};

`PopperVirtualElement` -> `PositioningVirtualElement`

Follows the Floating UI virtual element type. The only difference is that x & y are now required.

TS playground with example

`popperRef` -> `positioningRef`

Removes Popper instances from our API

Before

type PopperRefHandle = {
  updatePosition: () => void;
  setTarget: (target: HTMLElement | PositioningVirtualElement) => void;
};

<Popover positioning={{ popperRef }} />;

After

type PositioningRefHandle = {
  updatePosition: () => void;
  setTarget: (target: HTMLElement | PositioningVirtualElement) => void;
};
<Popover positioning={{ positioningRef }} />;

`PopperVirtualElement` -> `PositioningVirtualElement`

Type was renamed.

Fixes #22585

Updates `@fluentui/react-positioning` so that none of the popper API and types leak anymore

codesandbox-ci · 2022-04-21T15:38:01Z

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 d5505bf:

Sandbox	Source
@fluentui/react 8 starter	Configuration
@fluentui/react-components 9 starter	Configuration

fabricteam · 2022-04-21T15:39:22Z

📊 Bundle size report

Package & Exports	Baseline (minified/GZIP)	PR	Change
`react-combobox` Combobox	`60.792 kB` `20.778 kB`	`61.406 kB` `20.981 kB`	`614 B` `203 B`
`react-components` react-components: Accordion, Button, FluentProvider, Image, Menu, Popover	`180.24 kB` `50.256 kB`	`180.903 kB` `50.469 kB`	`663 B` `213 B`
`react-menu` Menu (including children components)	`110.243 kB` `33.543 kB`	`110.869 kB` `33.746 kB`	`626 B` `203 B`
`react-menu` Menu (including selectable components)	`113.418 kB` `34.011 kB`	`114.044 kB` `34.213 kB`	`626 B` `202 B`
`react-popover` Popover	`101.158 kB` `30.817 kB`	`101.821 kB` `31.067 kB`	`663 B` `250 B`
`react-positioning` usePositioning	`23.21 kB` `8.084 kB`	`23.828 kB` `8.283 kB`	`618 B` `199 B`
`react-tooltip` Tooltip	`42.939 kB` `14.782 kB`	`43.598 kB` `14.994 kB`	`659 B` `212 B`

Unchanged fixtures

Package & Exports	Size (minified/GZIP)
`react-avatar` Avatar	`45.608 kB` `13.312 kB`
`react-components` react-components: FluentProvider & webLightTheme	`34.078 kB` `11.098 kB`

_{🤖 This report was generated against 51777323141411ce197a6314e211c326af180c60}

fabricteam · 2022-04-21T15:41:24Z

Perf Analysis (`@fluentui/react-components`)

Scenario	Render type	Master Ticks	PR Ticks	Iterations	Status
FluentProviderWithTheme	mount	388	384	10	Possible regression
FluentProviderWithTheme	virtual-rerender	359	346	10	Possible regression
FluentProviderWithTheme	virtual-rerender-with-unmount	405	428	10	Possible regression

All results

Scenario	Render type	Master Ticks	PR Ticks	Iterations	Status
Avatar	mount	1265	1249	5000
Button	mount	760	793	5000
FluentProvider	mount	2305	2375	5000
FluentProviderWithTheme	mount	388	384	10	Possible regression
FluentProviderWithTheme	virtual-rerender	359	346	10	Possible regression
FluentProviderWithTheme	virtual-rerender-with-unmount	405	428	10	Possible regression
MakeStyles	mount	2031	1980	50000

size-auditor · 2022-04-21T15:42:51Z

Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: 51777323141411ce197a6314e211c326af180c60 (build)

layershifter · 2022-05-02T08:09:09Z

@@ -0,0 +1,7 @@
+{
+  "type": "prerelease",
+  "comment": "BREAKING CHANGE: Encapsulate Popper API",


Suggested change

"comment": "BREAKING CHANGE: Encapsulate Popper API",

"comment": "BREAKING: Rename types and APIs to avoid references to third party API",

?

layershifter · 2022-05-02T08:11:19Z

+  positionedElement: Rect;
+  target: Rect;
+  position: Position;
+  alignment?: Alignment;


Is it really optional?

yes, by default our alignment is center

spmonahan

Just putting a block here so we can coordinate merging breaking changes.

behowell

The react-combobox and react-tooltip changes look good, just a few nitpicks. I only skimmed the rest.

behowell · 2022-05-19T20:06:02Z

@@ -55,7 +55,7 @@ export const useCombobox_unstable = (props: ComboboxProps, ref: React.Ref<HTMLBu
  const popperOptions = {


(nit) rename popperOptions to positioningOptions

Ditto in other places that have a local popperOptions or popperState

makes sense done in d5505bf

…ox/useCombobox.ts Co-authored-by: Ben Howell <[email protected]>

…/useTooltip.tsx Co-authored-by: Ben Howell <[email protected]>

GeoffCoxMSFT · 2022-05-20T16:55:15Z

+export type Boundary = HTMLElement | Array<HTMLElement> | 'clippingParents' | 'scrollParent' | 'window';

-export type PopperRefHandle = {
+export type PositioningImperativeRef = {


Nit: Consider PositioningRef. Is the word imperative necessary? Aren't all ref's imperative? :) Do you think there will be other kinds of positioning refs that we'll need to distinguish between?

GeoffCoxMSFT · 2022-05-20T16:59:32Z

    position: 'below' as const,
    align: 'start' as const,
-    offset: [0, 2] as Offset,
+    offset: 2,


Should this change be to use the crossAxis, mainAxis offset instead of the single number?

GeoffCoxMSFT · 2022-05-20T17:03:13Z


-export type { PositioningProps, PositioningShorthand, PositioningShorthandValue } from '@fluentui/react-positioning';
+export type {
+  PositioningProps,


Do you also want to export the types used within these types such as Offset, Alignment, Position?

GeoffCoxMSFT · 2022-05-20T17:08:00Z

-import * as PopperJs from '@popperjs/core';
 import * as React from 'react';

+type Rect = {


Consider if you DOMRect would be a good structure to use here. It has x, y, width, and height, but also top, left, right, and bottom. It can be annoying to create instances of a DOMRect because of the fromRect method, but might fit well with positioning.

GeoffCoxMSFT · 2022-05-20T17:11:18Z

+ */
+export function parsePopperPlacement(placement: Placement): { basePlacement: BasePlacement; alignment: Variation } {
+  const tokens = placement.split('-');
+  return {


Is there any way that the placement could be malformed and not have 2 tokens? If so, consider a check here to either throw a specific error, provide a default, or return null.

GeoffCoxMSFT · 2022-05-20T17:12:47Z

+    ['top', undefined, 'above'],
+    ['bottom', undefined, 'below'],
+    ['left', undefined, 'before'],
+    ['right', undefined, 'after'],


Consider adding invalid input tests.

GeoffCoxMSFT · 2022-05-20T17:14:48Z

    [
-      [0, 0],
-      [0, 1],
+      { crossAxis: 0, mainAxis: 0 },


Can offsets be negative? What is the expected result if they are?

GeoffCoxMSFT · 2022-05-20T17:17:37Z

-    offset[1] += arrowHeight;
-  } else {
-    offset[1] = arrowHeight;
+const addArrowOffset = (offset: OffsetObject | number, arrowHeight: number): OffsetObject => {


Nit: While typescript will handle it, it is generally better js practice to always put method declarations before you use them when possible.

* BREAKING CHANGE: Encapsulate Popper API Updates `@fluentui/react-positioning` so that none of the popper API and types leak anymore * changefiles * update test * update stories * positioned to positionedElement * update api * remove getBasePlacement * add tests * virtual element and boundary * fix types * PR fixes * update md * fix type * changefile * Update packages/react-components/react-combobox/src/components/Combobox/useCombobox.ts Co-authored-by: Ben Howell <[email protected]> * Update packages/react-components/react-tooltip/src/components/Tooltip/useTooltip.tsx Co-authored-by: Ben Howell <[email protected]> * rename popperOptions to positioningOptions Co-authored-by: Ben Howell <[email protected]>

* react-button: Removing deprecated block prop from all button components (#23071) * react-button: Removing deprecated block prop from all button components. * Removing block stories. * Adding changefile. * Removing block prop from migration guide and vr tests. * fix(react-text): Update typography wrappers to latest naming (#22934) * fix: change Headline to Subtitle1 * fix: change Subheadline to Subtitle2 * fix: change Body to Body1 * fix: change Caption to Caption1 * feat: add Caption2 * chore: use better vr story name * chore: add change files * chore: update api * fix: rename components in missing story * feat: removed unused `caption2ClassName` * chore: disable classname conformance test * chore: remove deprecated className * chore: add conformance test exception * BREAKING: Removing deprecated `<componentName>ClassName` exports from v9 packages (#23026) * removing single static classnames * api files * change files * disabling component-has-static-classname-exported test in react-image * preparing to merge master * disabling export tests * removing change file for text * change file * disabling test in Dialog * disabling more tests * remove react-text class names Co-authored-by: Sean Monahan <[email protected]> * BREAKING: Update string unions to `spinal-case` (#23001) * react-avatar: update unions to use spinal-case BREAKING Per #22596 all string unions are being updated to use spinal-case * react-input: update unions to use spinal-case BREAKING Per #22596 all string unions are being updated to use spinal-case * react-radio: update unions to use spinal-case BREAKING Per #22596 all string unions are being updated to use spinal-case * react-select: update unions to use spinal-case BREAKING Per #22596 all string unions are being updated to use spinal-case * react-spinbutton: update unions to use spinal-case BREAKING Per #22596 all string unions are being updated to use spinal-case * react-tabster: update unions to use spinal-case BREAKING Per #22596 all string unions are being updated to use spinal-case * react-textarea: update unions to use spinal-case BREAKING Per #22596 all string unions are being updated to use spinal-case * react-badge: update unions to use spinal-case BREAKING Per #22596 all string unions are being updated to use spinal-case * react-combobox: update unions to use spinal-case BREAKING Per #22596 all string unions are being updated to use spinal-case * add change files for spinal-case update * react-card: update tabster tab behaviors Updates the `useCard()` implementation after the `react-tabster` spinal-case breaking change. * vr-tests: update types for v9 tests * react-avatar: update badge types in stories Avatar stories were using the old, camelCase PresenceBadge types. * react-badge: fix style that broke with spinal-case change One of the "doNotDisturb" cases was missed due to the non-typesafe way we tested the PresenceBadge status in styles. This test has been changed to a typesafe function call instead. * react-combobox: update props in appearance story Change filledDarker and filledLighter to filled-darker and filled-lighter respectively. * react-badge: update camelCase to spinal-case for strings * fix: missing inputClassName in merge conflict resolution * react-select: update api snapshot * react-alert: update conformance tests * fix: update react-components api snapshot * BREAKING CHANGE: Encapsulate Popper API (#22586) * BREAKING CHANGE: Encapsulate Popper API Updates `@fluentui/react-positioning` so that none of the popper API and types leak anymore * changefiles * update test * update stories * positioned to positionedElement * update api * remove getBasePlacement * add tests * virtual element and boundary * fix types * PR fixes * update md * fix type * changefile * Update packages/react-components/react-combobox/src/components/Combobox/useCombobox.ts Co-authored-by: Ben Howell <[email protected]> * Update packages/react-components/react-tooltip/src/components/Tooltip/useTooltip.tsx Co-authored-by: Ben Howell <[email protected]> * rename popperOptions to positioningOptions Co-authored-by: Ben Howell <[email protected]> * BREAKING: Remove context exports from react-components (#23085) * BREAKING: Remove context exports from react-components In order to provider proper encapsulation and allow us to change internals more easily, this PR removes exports of context objects from react-components. In cases where contexts were removed, the export was replaced with a specific provider component and hook to use the context value. Follows from #22300 * changefile * stop exporting menu contexts * changefile * update md Co-authored-by: Sean Monahan <[email protected]> * react-popover: update api snapshot * BREAKING(react-theme): remove webHighContrastTheme (#23083) * fix(react-theme): Remove webHighContrast theme Co-authored-by: Makoto Morimoto <[email protected]> Co-authored-by: André Dias (he/him) <[email protected]> Co-authored-by: Esteban Munoz Facusse <[email protected]> Co-authored-by: ling1726 <[email protected]> Co-authored-by: Ben Howell <[email protected]> Co-authored-by: Miroslav Stastny <[email protected]>

* react-button: Removing deprecated block prop from all button components (microsoft#23071) * react-button: Removing deprecated block prop from all button components. * Removing block stories. * Adding changefile. * Removing block prop from migration guide and vr tests. * fix(react-text): Update typography wrappers to latest naming (microsoft#22934) * fix: change Headline to Subtitle1 * fix: change Subheadline to Subtitle2 * fix: change Body to Body1 * fix: change Caption to Caption1 * feat: add Caption2 * chore: use better vr story name * chore: add change files * chore: update api * fix: rename components in missing story * feat: removed unused `caption2ClassName` * chore: disable classname conformance test * chore: remove deprecated className * chore: add conformance test exception * BREAKING: Removing deprecated `<componentName>ClassName` exports from v9 packages (microsoft#23026) * removing single static classnames * api files * change files * disabling component-has-static-classname-exported test in react-image * preparing to merge master * disabling export tests * removing change file for text * change file * disabling test in Dialog * disabling more tests * remove react-text class names Co-authored-by: Sean Monahan <[email protected]> * BREAKING: Update string unions to `spinal-case` (microsoft#23001) * react-avatar: update unions to use spinal-case BREAKING Per microsoft#22596 all string unions are being updated to use spinal-case * react-input: update unions to use spinal-case BREAKING Per microsoft#22596 all string unions are being updated to use spinal-case * react-radio: update unions to use spinal-case BREAKING Per microsoft#22596 all string unions are being updated to use spinal-case * react-select: update unions to use spinal-case BREAKING Per microsoft#22596 all string unions are being updated to use spinal-case * react-spinbutton: update unions to use spinal-case BREAKING Per microsoft#22596 all string unions are being updated to use spinal-case * react-tabster: update unions to use spinal-case BREAKING Per microsoft#22596 all string unions are being updated to use spinal-case * react-textarea: update unions to use spinal-case BREAKING Per microsoft#22596 all string unions are being updated to use spinal-case * react-badge: update unions to use spinal-case BREAKING Per microsoft#22596 all string unions are being updated to use spinal-case * react-combobox: update unions to use spinal-case BREAKING Per microsoft#22596 all string unions are being updated to use spinal-case * add change files for spinal-case update * react-card: update tabster tab behaviors Updates the `useCard()` implementation after the `react-tabster` spinal-case breaking change. * vr-tests: update types for v9 tests * react-avatar: update badge types in stories Avatar stories were using the old, camelCase PresenceBadge types. * react-badge: fix style that broke with spinal-case change One of the "doNotDisturb" cases was missed due to the non-typesafe way we tested the PresenceBadge status in styles. This test has been changed to a typesafe function call instead. * react-combobox: update props in appearance story Change filledDarker and filledLighter to filled-darker and filled-lighter respectively. * react-badge: update camelCase to spinal-case for strings * fix: missing inputClassName in merge conflict resolution * react-select: update api snapshot * react-alert: update conformance tests * fix: update react-components api snapshot * BREAKING CHANGE: Encapsulate Popper API (microsoft#22586) * BREAKING CHANGE: Encapsulate Popper API Updates `@fluentui/react-positioning` so that none of the popper API and types leak anymore * changefiles * update test * update stories * positioned to positionedElement * update api * remove getBasePlacement * add tests * virtual element and boundary * fix types * PR fixes * update md * fix type * changefile * Update packages/react-components/react-combobox/src/components/Combobox/useCombobox.ts Co-authored-by: Ben Howell <[email protected]> * Update packages/react-components/react-tooltip/src/components/Tooltip/useTooltip.tsx Co-authored-by: Ben Howell <[email protected]> * rename popperOptions to positioningOptions Co-authored-by: Ben Howell <[email protected]> * BREAKING: Remove context exports from react-components (microsoft#23085) * BREAKING: Remove context exports from react-components In order to provider proper encapsulation and allow us to change internals more easily, this PR removes exports of context objects from react-components. In cases where contexts were removed, the export was replaced with a specific provider component and hook to use the context value. Follows from microsoft#22300 * changefile * stop exporting menu contexts * changefile * update md Co-authored-by: Sean Monahan <[email protected]> * react-popover: update api snapshot * BREAKING(react-theme): remove webHighContrastTheme (microsoft#23083) * fix(react-theme): Remove webHighContrast theme Co-authored-by: Makoto Morimoto <[email protected]> Co-authored-by: André Dias (he/him) <[email protected]> Co-authored-by: Esteban Munoz Facusse <[email protected]> Co-authored-by: ling1726 <[email protected]> Co-authored-by: Ben Howell <[email protected]> Co-authored-by: Miroslav Stastny <[email protected]>

BREAKING CHANGE: Encapsulate Popper API

f81efa4

Updates `@fluentui/react-positioning` so that none of the popper API and types leak anymore

msft-fluent-ui-bot assigned miroslavstastny Apr 21, 2022

changefiles

4a464f2

ling1726 added 4 commits April 21, 2022 18:23

update test

af61470

update stories

94feb0d

positioned to positionedElement

97c8f09

update api

b009675

ling1726 marked this pull request as ready for review April 21, 2022 18:10

ling1726 requested review from a team, behowell, khmakoto and smhigley as code owners April 21, 2022 18:10

ling1726 added 4 commits April 21, 2022 20:19

remove getBasePlacement

61a59b4

add tests

5844d90

virtual element and boundary

d0831a9

fix types

28ea44b

JustSlone mentioned this pull request Apr 27, 2022

Coordinate Breaking RC Changes #22130

Closed

Merge branch 'master' into feat/popper-api-refactor

064463a

layershifter reviewed May 2, 2022

View reviewed changes

ling1726 added 4 commits May 4, 2022 13:14

merge master

f8ee08d

PR fixes

2f093fe

update md

302b92d

fix type

5938b33

ling1726 added 2 commits May 18, 2022 19:55

merge master

e27f5f3

changefile

a30b30d

spmonahan requested changes May 18, 2022

View reviewed changes

msft-fluent-ui-bot added the Needs: Author Feedback label May 18, 2022

merge master

b80803f

msft-fluent-ui-bot removed the Needs: Author Feedback label May 18, 2022

spmonahan changed the base branch from master to 9.0-breaking-changes May 19, 2022 19:25

behowell approved these changes May 19, 2022

View reviewed changes

spmonahan changed the base branch from 9.0-breaking-changes to master May 20, 2022 01:22

ling1726 and others added 3 commits May 20, 2022 08:41

Update packages/react-components/react-combobox/src/components/Combob…

68bad26

…ox/useCombobox.ts Co-authored-by: Ben Howell <[email protected]>

Update packages/react-components/react-tooltip/src/components/Tooltip…

70804c1

…/useTooltip.tsx Co-authored-by: Ben Howell <[email protected]>

rename popperOptions to positioningOptions

d5505bf

layershifter approved these changes May 20, 2022

View reviewed changes

GeoffCoxMSFT reviewed May 20, 2022

View reviewed changes

GeoffCoxMSFT approved these changes May 20, 2022

View reviewed changes

spmonahan changed the base branch from master to 9.0-breaking-changes May 20, 2022 18:24

spmonahan merged commit 968737e into microsoft:9.0-breaking-changes May 20, 2022

layershifter mentioned this pull request May 24, 2022

docs: cleanup docs pages #23187

Merged

layershifter mentioned this pull request Aug 8, 2022

Floating UI breaking changes #22585

Closed

	"comment": "BREAKING CHANGE: Encapsulate Popper API",
	"comment": "BREAKING: Rename types and APIs to avoid references to third party API",

		@@ -55,7 +55,7 @@ export const useCombobox_unstable = (props: ComboboxProps, ref: React.Ref<HTMLBu
		const popperOptions = {

Uh oh!

Conversation

ling1726 commented Apr 21, 2022 • edited by GeoffCoxMSFT Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ BREAKING CHANGES

Offset array -> Offset object

Before

After

Offset function argument

Before

After

PopperVirtualElement -> PositioningVirtualElement

popperRef -> positioningRef

Before

After

PopperVirtualElement -> PositioningVirtualElement

Uh oh!

codesandbox-ci Bot commented Apr 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fabricteam commented Apr 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Bundle size report

Uh oh!

fabricteam commented Apr 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

size-auditor Bot commented Apr 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Asset size changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

spmonahan left a comment

Choose a reason for hiding this comment

Uh oh!

behowell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

ling1726 commented Apr 21, 2022 •

edited by GeoffCoxMSFT

Loading

`PopperVirtualElement` -> `PositioningVirtualElement`

`popperRef` -> `positioningRef`

`PopperVirtualElement` -> `PositioningVirtualElement`

codesandbox-ci Bot commented Apr 21, 2022 •

edited

Loading

fabricteam commented Apr 21, 2022 •

edited

Loading

fabricteam commented Apr 21, 2022 •

edited

Loading

size-auditor Bot commented Apr 21, 2022 •

edited

Loading