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(Types): ShorthandCollection #13050

Merged
merged 44 commits into from
May 21, 2020

Conversation

assuncaocharles
Copy link
Contributor

@assuncaocharles assuncaocharles commented May 7, 2020

Pull request checklist

Description of changes

In #1605 was done an update for ShorthandCollection but it was not fully complete:

export interface ToolbarProps {
  /** Shorthand array of props for Toolbar. */
  items?: ShorthandCollection<ToolbarItemProps, ToolbarItemShorthandKinds>
}

It's not enough because for kind="divider" we should have ToolbarDividerProps.

This PR implements the proposed solution:

export interface ToolbarProps {
  /** Shorthand array of props for Toolbar. */
  items?: ShorthandCollection<ToolbarItemProps,  {
    item: ToolbarItemProps,
    divider: ToolbarDividerProps
  }>
}

To achieve that the ShorthandCollection type had to be changed to:

type ShorthandCollection<P, K = Record<string, {}>> = ShorthandValue<
  P & { kind?: keyof K } & Partial<K[keyof K]>
>[];

And alternative solution that can better is to create a new type:

type ShorthandCollection<P, K = never> = ShorthandValue<P>[]

type ShorthandCollectionWithKind<P, K = never> = ShorthandValue<
  P & { kind?: keyof K } & { K[keyof K] }
>[]

And then replace the places where ShorthandCollection are used together with kind.

Focus areas to test

(optional)

Microsoft Reviewers: Open in CodeFlow
Microsoft Reviewers: Open in CodeFlow

@size-auditor
Copy link

size-auditor bot commented May 7, 2020

Asset size changes

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

Baseline commit: 73cbfd0c0d41af4839d32070f2b6134cc4b3f471 (build)

@assuncaocharles assuncaocharles changed the title Fix/type shorthand fix(Types): ShorthandCollection May 7, 2020
@assuncaocharles
Copy link
Contributor Author

@layershifter

export type ShorthandCollection<P, K = Record<string, {}>> = ShorthandValue<
  P & { kind?: keyof K } & Partial<K[keyof K]>
>[];

It works, what you think about?

…ix/type-shorthand

� Conflicts:
�	packages/fluentui/react-northstar/src/components/Menu/Menu.tsx
@assuncaocharles
Copy link
Contributor Author

@layershifter added the solution we agreed.

@assuncaocharles assuncaocharles merged commit 57c30dd into microsoft:master May 21, 2020
miroslavstastny pushed a commit to levithomason/fluentui that referenced this pull request Jun 8, 2020
* fix(Types): fix shorthandCollection Definition

* fix(Types): add never

* fix(Types): key in keyof

* fix(types): shorthand

* fix(types): optional

* fix(Types): fix shorthandCollection Definition

* fix(Types): add never

* fix(Types): key in keyof

* fix(types): shorthand

* fix(types): optional

* fix(Types): Partial typ

* fix(Types): Fix toolbar usage

* fix(Types): remove log

* fix(Types): UIComponentProps

* fix(Types): Shorthand type

* fix(Types): Simplify

* Update packages/fluentui/docs/src/examples/components/Toolbar/Performance/CustomToolbar.perf.tsx

Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>

* Revert "Update packages/fluentui/docs/src/examples/components/Toolbar/Performance/CustomToolbar.perf.tsx"

This reverts commit 6d1e555.

* fix(Types): change type

* fix(Types): fix usage

* fix(types): fix usage

* fix(types): Using PROPS

* fix(types): Fix import

* fix(types): fix usage

* fix(types): fix usage

* fix(Types): change default generic

* chore(types): fix usage

* improve typings in Toolbar

* restore Dropdown to master

* fix typings issues

* prettify example

* remove useless object parens

* fix(types): kind selector

* fix(types): add shorthand value

* fix(Types): change items type in example

* fix(types): kind optinal

* fix(types): fix

Co-authored-by: Oleksandr Fediashov <alexander.mcgarret@gmail.com>
Co-authored-by: Oleksandr Fediashov <olfedias@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fluent UI react-northstar (v0) Work related to Fluent UI V0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants