feat(react-utilities): restricts trigger API types #25044
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
📊 Bundle size report
Unchanged fixtures
|
|
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 796bbc3:
|
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: 0afe50e55c572348c93928dd9883cef537bb8101 (build) |
Collaborator
Perf Analysis (
|
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| Avatar | mount | 1301 | 1268 | 5000 | |
| Button | mount | 921 | 928 | 5000 | |
| FluentProvider | mount | 1500 | 1479 | 5000 | |
| FluentProviderWithTheme | mount | 577 | 577 | 10 | |
| FluentProviderWithTheme | virtual-rerender | 546 | 539 | 10 | |
| FluentProviderWithTheme | virtual-rerender-with-unmount | 579 | 561 | 10 | |
| MakeStyles | mount | 1953 | 1955 | 50000 | |
| SpinButton | mount | 2379 | 2335 | 5000 |
khmakoto
reviewed
Oct 3, 2022
packages/react-components/react-dialog/src/components/DialogTrigger/useDialogTrigger.ts
Outdated
Show resolved
Hide resolved
packages/react-components/react-menu/src/components/MenuTrigger/MenuTrigger.types.ts
Outdated
Show resolved
Hide resolved
packages/react-components/react-popover/src/components/PopoverTrigger/PopoverTrigger.types.ts
Outdated
Show resolved
Hide resolved
packages/react-components/react-utilities/src/trigger/applyTriggerPropsToChildren.test.tsx
Outdated
Show resolved
Hide resolved
packages/react-components/react-utilities/src/trigger/applyTriggerPropsToChildren.ts
Outdated
Show resolved
Hide resolved
packages/react-components/react-utilities/src/trigger/getTriggerChild.test.tsx
Outdated
Show resolved
Hide resolved
packages/react-components/react-utilities/src/trigger/getTriggerChild.ts
Outdated
Show resolved
Hide resolved
packages/react-components/react-utilities/src/trigger/isFluentTrigger.ts
Outdated
Show resolved
Hide resolved
bsunderhus
force-pushed
the
feat(react-utilities)--improve-trigger-types
branch
from
7a2fc1a to
7823757
Compare
layershifter
reviewed
Oct 4, 2022
packages/react-components/react-utilities/src/trigger/getTriggerChild.ts
Outdated
Show resolved
Hide resolved
packages/react-components/react-utilities/etc/react-utilities.api.md
Outdated
Show resolved
Hide resolved
packages/react-components/react-utilities/etc/react-utilities.api.md
Outdated
Show resolved
Hide resolved
packages/react-components/react-popover/src/components/PopoverTrigger/usePopoverTrigger.ts
Outdated
Show resolved
Hide resolved
change/@fluentui-react-tooltip-40830067-8e20-4a61-9deb-91660edd6e2c.json
Outdated
Show resolved
Hide resolved
bsunderhus
force-pushed
the
feat(react-utilities)--improve-trigger-types
branch
from
7823757 to
796bbc3
Compare
layershifter
approved these changes
Oct 4, 2022
khmakoto
approved these changes
Oct 4, 2022
| import { useModalAttributes } from '@fluentui/react-tabster'; | ||
| import { applyTriggerPropsToChildren, getTriggerChild, useEventCallback } from '@fluentui/react-utilities'; | ||
| import { DialogTriggerChildProps, DialogTriggerProps, DialogTriggerState } from './DialogTrigger.types'; | ||
| import { DialogTriggerProps, DialogTriggerState } from './DialogTrigger.types'; |
Member
There was a problem hiding this comment.
Suggested change
| import { DialogTriggerProps, DialogTriggerState } from './DialogTrigger.types'; | |
| import type { DialogTriggerProps, DialogTriggerState } from './DialogTrigger.types'; |
| import { usePopoverContext_unstable } from '../../popoverContext'; | ||
| import type { PopoverTriggerChildProps, PopoverTriggerProps, PopoverTriggerState } from './PopoverTrigger.types'; | ||
| import type { PopoverTriggerProps, PopoverTriggerState } from './PopoverTrigger.types'; | ||
| import { useARIAButtonProps } from '@fluentui/react-aria'; |
Member
There was a problem hiding this comment.
Suggested change
| import { useARIAButtonProps } from '@fluentui/react-aria'; | |
| import type { useARIAButtonProps } from '@fluentui/react-aria'; |
marcosmoura
added a commit
to marcosmoura/fluentui
that referenced
this pull request
Oct 5, 2022
* master: (62 commits) chore(migrate-converged): add functionality to execute various v9 package file restructuring tasks (microsoft#24458) chore(react-dialog): updates stories (microsoft#25070) refactor Progress to remove label and description slots (microsoft#25067) fix(SplitButton): Remove borders from hover and pressed state in primary split buttons (microsoft#25059) chore(react-persona): Add vr, conformance, and unit tests (microsoft#25007) (docs): update Icon docs with examples (microsoft#24768) Split button/primary hc fix (microsoft#25066) chore(react-utilities): restricts trigger API types (microsoft#25044) Add utilities export @fluentui/style-utilities v8 (microsoft#25065) chore: migrate whole repo to webpack 5 (microsoft#24542) applying package updates BREAKING: refactor `useTable` to be composable (microsoft#24947) Added shims to shim packages (microsoft#25048) Add Progress SPEC (microsoft#24418) applying package updates chore(react-persona): Add bundle-size command for bundle-size fixtures (microsoft#25055) fix(Button): Remove margin added to buttons by Safari (microsoft#25052) fix: Menu triggers no longer take focus when they are mounted (microsoft#25016) chore: deletes react-trigger package (microsoft#25042) Fixed a minor typo: immmediately => immediately (microsoft#25036) ...
NotWoods
pushed a commit
to NotWoods/fluentui
that referenced
this pull request
Nov 18, 2022
ling1726
added a commit
to ling1726/fluentui
that referenced
this pull request
Sep 26, 2023
Fixes regression from microsoft#28530 The `contextMenuProps` aren't really for just the context menu but common props for all menus fter the refactor in microsoft#25044. We should probably revisit the menu trigger code and clean it up but this fix is more urgent
ling1726
added a commit
that referenced
this pull request
Sep 26, 2023
* fix: Context menu close should restore focus Fixes regression from #28530 The `contextMenuProps` aren't really for just the context menu but common props for all menus fter the refactor in #25044. We should probably revisit the menu trigger code and clean it up but this fix is more urgent * changefile * revert * revert
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Current Behavior
Currently types by trigger API are quite loose, on some cases such as on
getTriggerChildmethod a generic type would be required to ensure typings.New Behavior
Makes types stricter providing a well defined
TriggerPropstypes ensuring that helper methods such asgetTriggerChildcan properly infer types without the requirement of generics being necessarily provided.