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

feat(MessageBar): Initial implementation #29313

Merged

Conversation

ling1726
Copy link
Member

@ling1726 ling1726 commented Sep 27, 2023

Adds an initial implementation of MessageBar that includes design for different intents and multiline handling.

image

image

Addresses: #22579

Adds an initial implementation of MessageBar that includes design for
different intents and multiline handling.
@ling1726 ling1726 marked this pull request as ready for review September 27, 2023 09:08
@ling1726 ling1726 requested a review from a team as a code owner September 27, 2023 09:08
@fabricteam
Copy link
Collaborator

fabricteam commented Sep 27, 2023

📊 Bundle size report

🤖 This report was generated against e7c0c14830fc2a29b66c4433bce8e7dde7eceba0

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 27, 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 eaca078:

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

@size-auditor
Copy link

size-auditor bot commented Sep 27, 2023

Asset size changes

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

Baseline commit: e7c0c14830fc2a29b66c4433bce8e7dde7eceba0 (build)

@layershifter layershifter changed the title feat: Initial implementation feat(MessageBar): Initial implementation Sep 27, 2023
@ling1726 ling1726 requested a review from a team as a code owner September 27, 2023 15:47
@ling1726 ling1726 removed the request for review from a team September 27, 2023 15:50
@ling1726 ling1726 closed this Sep 28, 2023
@ling1726 ling1726 reopened this Sep 28, 2023
ling1726 added a commit to ling1726/fluentui that referenced this pull request Sep 28, 2023
Cypress test config previously did not use TsConfigPathsPlugin so it
relied on code being built in CI.

However there is an edge case encountered in microsoft#29313. Lage will run the
cypress tests of depednencies which *_can depend on depedencies not in
the package.json tree_* 💣💣

```
- react-message-bar-preview
  - react-button
    - react-tabster
      - react-provider (not in package.json, but used in cypress test)
```
@ling1726 ling1726 force-pushed the react-message-bar/feat/initial-implementation branch from 05c1cd2 to e328ff0 Compare September 28, 2023 14:14
@fabricteam
Copy link
Collaborator

Perf Analysis (@fluentui/react-components)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 634 616 5000
Button mount 309 315 5000
Field mount 1111 1111 5000
FluentProvider mount 680 691 5000
FluentProviderWithTheme mount 78 85 10
FluentProviderWithTheme virtual-rerender 65 64 10
FluentProviderWithTheme virtual-rerender-with-unmount 74 71 10
InfoButton mount 10 10 5000
MakeStyles mount 856 858 50000
Persona mount 1775 1677 5000
SpinButton mount 1365 1370 5000

@fabricteam
Copy link
Collaborator

🕵 fluentuiv9 No visual regressions between this PR and main

ling1726 added a commit that referenced this pull request Sep 28, 2023
Cypress test config previously did not use TsConfigPathsPlugin so it
relied on code being built in CI.

However there is an edge case encountered in #29313. Lage will run the
cypress tests of depednencies which *_can depend on depedencies not in
the package.json tree_* 💣💣

```
- react-message-bar-preview
  - react-button
    - react-tabster
      - react-provider (not in package.json, but used in cypress test)
```
@ling1726 ling1726 merged commit 9fd9f70 into microsoft:master Sep 28, 2023
23 checks passed
*/
export type MessageBarProps = ComponentProps<MessageBarSlots> &
Pick<MessageBarContextValue, 'layout'> & {
multiline?: boolean;
Copy link
Member

Choose a reason for hiding this comment

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

Cruft?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah removed in #29328

Component: MessageBarActions,
displayName: 'MessageBarActions',
disabledTests: [
// TODO: having problems due to the fact root of DialogTitle is Fragment
Copy link
Member

Choose a reason for hiding this comment

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

What?

Copy link
Member Author

Choose a reason for hiding this comment

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

I copied that config from DialogTitle, will remove in another PR - the test itself should still be disabled tho

import type { MessageBarActionsProps } from './MessageBarActions.types';

/**
* MessageBarActions component - TODO: add more docs
Copy link
Member

Choose a reason for hiding this comment

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

Let's solve todo

/**
* MessageBarActions Props
*/
export type MessageBarActionsProps = ComponentProps<MessageBarActionsSlots> & {};
Copy link
Member

Choose a reason for hiding this comment

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

Let's remove {} as it's any

assertSlots<MessageBarActionsSlots>(state);
if (state.layout === 'multiline') {
return (
<ButtonContextProvider value={{ size: 'small' }}>
Copy link
Member

Choose a reason for hiding this comment

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

  • This should be a memoized value as ButtonContextProvider is not using context selectors
  • The value should be defined in useMessageBarContextValues

/**
* MessageBarBody Props
*/
export type MessageBarBodyProps = ComponentProps<MessageBarBodySlots> & {};
Copy link
Member

Choose a reason for hiding this comment

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

Let's remove {}

/**
* MessageBarTitle Props
*/
export type MessageBarTitleProps = ComponentProps<MessageBarTitleSlots> & {};
Copy link
Member

Choose a reason for hiding this comment

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

Let's remove {}

export const renderMessageBarTitle_unstable = (state: MessageBarTitleState) => {
assertSlots<MessageBarTitleSlots>(state);

// TODO Add additional slots in the appropriate place
Copy link
Member

Choose a reason for hiding this comment

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

Can be removed?


// TODO Add additional slots in the appropriate place
return (
<>
Copy link
Member

Choose a reason for hiding this comment

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

Useless Fragment

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.

None yet

4 participants