Skip to content

Make DocumentCardAction props extend from Button props. This will make sure we can set all the properties on the action button such as aria label. #170

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.

Sign up for GitHub

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

Merged
yiminwu merged 7 commits into from
Aug 22, 2016
Merged

Make DocumentCardAction props extend from Button props. This will make sure we can set all the properties on the action button such as aria label. #170

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
46f352c
add empty alt attribute to the image with presentation role in Docume…
yiminwu Aug 19, 2016
f4d7b73
Merge branch 'master' of https://github.com/OfficeDev/office-ui-fabri…
yiminwu Aug 19, 2016
d45f90a
Merge branch 'master' of https://github.com/OfficeDev/office-ui-fabri…
yiminwu Aug 19, 2016
af46bd6
Make DocumentCardAction props extend from Button props. This will mak…
yiminwu Aug 19, 2016
6e8f2f0
Add missing semicolon
yiminwu Aug 19, 2016
db8c528
remove IDocumentCardAction interface since we actually only need butt…
yiminwu Aug 20, 2016
83b036b
Merge branch 'master' into master
yiminwu Aug 22, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 2 additions & 13 deletions src/components/DocumentCard/DocumentCard.Props.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { DocumentCardActivity } from './DocumentCardActivity';
import { DocumentCardActions } from './DocumentCardActions';
import { PersonaInitialsColor } from '../../Persona';
import { ImageFit } from '../../Image';
import { IButtonProps } from '../../Button';

export interface IDocumentCardProps extends React.Props<DocumentCard> {
/**
Expand Down Expand Up @@ -145,22 +146,10 @@ export interface IDocumentCardActionsProps extends React.Props<DocumentCardActio
/**
* The actions available for this document.
*/
actions: IDocumentCardAction[];
actions: IButtonProps[];
Copy link
Member

@dzearing dzearing Aug 20, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

instead of changing the type here, maybe you should leave the previous and deprecate it.

add "buttons" to the contract as an optional.

Then you are not breaking existing people. We need to avoid breaks as much as possible.

Or maybe don't change it, extend action definition as needed.

Copy link
Contributor

@cliffkoh cliffkoh Aug 20, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

BTW, this actually only breaks people who imports IDocumentCardAction.. anyone who specifies actions: { ... } without importing the type will not be broken...

We could of course leave IDocumentCardAction in and while I don't think you can deprecate an interface, you could possibly deprecate both properties and indicate the entire interface is deprecated.

Copy link
Member

@dzearing dzearing Aug 20, 2016

Choose a reason for hiding this comment

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

ok maybe it's ok :)

Approved


/**
* The number of views this document has received.
*/
views?: Number;
}

export interface IDocumentCardAction {
/**
* The icon for this action.
*/
icon: string;

/**
* Function to run when clicking the action.
*/
onClick?: (ev?: any) => void;
}
18 changes: 8 additions & 10 deletions src/components/DocumentCard/DocumentCardActions.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,14 @@ export class DocumentCardActions extends React.Component<IDocumentCardActionsPro
return (
<div className='ms-DocumentCardActions'>

{ actions && actions.map((action, index) => (
<div className='ms-DocumentCardActions-action' key={ index }>
<Button
buttonType={ ButtonType.icon }
icon={ action.icon }
onClick={ action.onClick }
rootProps={ { title:'' } }
description='' />
</div>
)) }
{ actions && actions.map((action, index) => {
action.buttonType = ButtonType.icon;
return (
<div className='ms-DocumentCardActions-action' key={ index }>
<Button { ...action } />
</div>
);
}) }

{ views && (
<div className='ms-DocumentCardActions-views'>
Expand Down
47 changes: 28 additions & 19 deletions src/demo/pages/DocumentCardPage/examples/DocumentCard.Complete.Example.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,28 +62,37 @@ export class DocumentCardCompleteExample extends React.Component<any, any> {
}
/>
<DocumentCardActions
actions={
[
{ icon: 'share', onClick: (ev: any) => {
console.log('You clicked the share action.');
ev.preventDefault();
ev.stopPropagation();
}
actions={
[
{
icon: 'share',
onClick: (ev: any) => {
console.log('You clicked the share action.');
ev.preventDefault();
ev.stopPropagation();
},
{ icon: 'pinLeft', onClick: (ev: any) => {
console.log('You clicked the pin action.');
ev.preventDefault();
ev.stopPropagation();
}
ariaLabel: 'share action'
},
{
icon: 'pinLeft',
onClick: (ev: any) => {
console.log('You clicked the pin action.');
ev.preventDefault();
ev.stopPropagation();
},
{ icon: 'bell', onClick: (ev: any) => {
console.log('You clicked the bell action.');
ev.preventDefault();
ev.stopPropagation();
}
ariaLabel: 'pin left action'
},
{
icon: 'bell',
onClick: (ev: any) => {
console.log('You clicked the bell action.');
ev.preventDefault();
ev.stopPropagation();
},
]
}
ariaLabel: 'bell action'
},
]
}
views={ 432 }
/>
</DocumentCard>
Expand Down