-
Notifications
You must be signed in to change notification settings - Fork 78
Fixed and updated calls of Primary/Default buttons for Chat, Calling and Storybook #883
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
Changes from all commits
a63ee77
da17e3d
956e1ac
df7821e
6727fa9
83b697e
ce46b48
2d1b129
5489faf
9b3c40d
c20143e
9bf4027
77c600c
27b55b2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| { | ||
| "type": "none", | ||
| "comment": "updated call of Primary/Default buttons", | ||
| "packageName": "@internal/react-components", | ||
| "email": "alcail@microsoft.com", | ||
| "dependentChangeType": "none" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| { | ||
| "type": "none", | ||
| "comment": "updated call of Primary/Default buttons", | ||
| "packageName": "@internal/react-composites", | ||
| "email": "alcail@microsoft.com", | ||
| "dependentChangeType": "none" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| { | ||
| "type": "none", | ||
| "comment": "updated call of Primary/Default buttons", | ||
| "packageName": "@internal/storybook", | ||
| "email": "alcail@microsoft.com", | ||
| "dependentChangeType": "none" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,7 @@ | |
|
|
||
| import { CSSProperties } from 'react'; | ||
| import { ComponentSlotStyle } from '@fluentui/react-northstar'; | ||
| import { mergeStyles } from '@fluentui/react'; | ||
| import { IButtonStyles, mergeStyles } from '@fluentui/react'; | ||
|
|
||
| /** | ||
| * @private | ||
|
|
@@ -145,6 +145,15 @@ export const newMessageButtonStyle = mergeStyles({ | |
| width: 'fit-content' | ||
| }); | ||
|
|
||
| /** | ||
| * @private | ||
| */ | ||
| export const buttonWithIconStyles: IButtonStyles = { | ||
| textContainer: { | ||
| display: 'contents' | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you double check if this works well with a11y? MDN says that child elements' aria tags might be skipped: https://developer.mozilla.org/en-US/docs/Web/CSS/display#box
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| } | ||
| }; | ||
|
|
||
| /** | ||
| * @private | ||
| */ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ import { | |
| endCallContainerStyle, | ||
| endCallTitleStyle, | ||
| buttonStyle, | ||
| buttonWithIconStyles, | ||
| mainStackTokens, | ||
| buttonsStackTokens, | ||
| upperStackTokens, | ||
|
|
@@ -36,10 +37,13 @@ export function Error(props: ErrorProps): JSX.Element { | |
| <Stack tokens={upperStackTokens}> | ||
| <div className={endCallTitleStyle}>{title}</div> | ||
| <Stack horizontal tokens={buttonsStackTokens}> | ||
| <PrimaryButton className={buttonStyle} onClick={props.rejoinHandler}> | ||
| <Video20Filled primaryFill="currentColor" className={videoCameraIconStyle} /> | ||
| {rejoinCall} | ||
| </PrimaryButton> | ||
| <PrimaryButton | ||
| className={buttonStyle} | ||
| styles={buttonWithIconStyles} | ||
| text={rejoinCall} | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also moved to locale strings file (maybe worth filing a bug with these instances in them) |
||
| onClick={props.rejoinHandler} | ||
| onRenderIcon={() => <Video20Filled className={videoCameraIconStyle} />} | ||
| /> | ||
| </Stack> | ||
| <div className={bottomStackFooterStyle}> | ||
| {props.reason ? ( | ||
|
|
||

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(unrelated to this PR) This needs moved to the locale strings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will assigned a task for myself to that after