Skip to content

Issue#4832: Break BaseButton Types hard dependency from class ContextualMenu #4845

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
antonlabunets merged 5 commits into from
May 11, 2018
Merged

Issue#4832: Break BaseButton Types hard dependency from class ContextualMenu #4845

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c2ce9d6
Breaking types dependency from ContextualMenu class
pchandoria May 10, 2018
8f12cf1
remove refernece from IContextualMenuSection as well
pchandoria May 10, 2018
fac2051
Merge branch 'master' of https://github.com/OfficeDev/office-ui-fabri…
pchandoria May 11, 2018
679a4a1
rush change
pchandoria May 11, 2018
fec4269
comment the code for usage of any to be removed soon
pchandoria May 11, 2018
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
11 changes: 11 additions & 0 deletions common/changes/office-ui-fabric-react/pchandoria-contextualMenu-fix_2018-05-11-00-54.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"changes": [
{
"packageName": "office-ui-fabric-react",
"comment": "Breaking BaseButton Types dependency from ContextualMenu class",
"type": "patch"
Copy link
Collaborator

@antonlabunets antonlabunets May 11, 2018

Choose a reason for hiding this comment

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

I'm not certain this is a patch. What if people used to depend on props from ContextualMenu? @dzearing / @aditima - what's your take. When I chatted with @pgonzal we believe this a breaking change that we may need to hold off till 6?

Copy link
Contributor

@joschect joschect May 11, 2018

Choose a reason for hiding this comment

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

They can still use them because it's any. This won't be breaking.

Copy link
Collaborator

@antonlabunets antonlabunets May 11, 2018

Choose a reason for hiding this comment

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

On second though any covers everything and the ContextMenu was itself pointing back to IContextualMenuProps

}
],
"packageName": "office-ui-fabric-react",
"email": "[email protected]"
}
11 changes: 8 additions & 3 deletions packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import * as React from 'react';
import { ContextualMenu } from './ContextualMenu';
import { DirectionalHint } from '../../common/DirectionalHint';
import { FocusZoneDirection, IFocusZoneProps } from '../../FocusZone';
import { IIconProps } from '../Icon/Icon.types';
Expand Down Expand Up @@ -29,7 +28,10 @@ export interface IContextualMenu {

}

export interface IContextualMenuProps extends React.Props<ContextualMenu>, IWithResponsiveModeState {
/**
* React.Props is deprecated and we're removing it in 6.0. Usage of 'any' should go away with it.
*/
export interface IContextualMenuProps extends React.Props<any>, IWithResponsiveModeState {
/**
* Optional callback to access the IContextualMenu interface. Use this instead of ref for accessing
* the public methods and properties of the component.
Expand Down Expand Up @@ -489,7 +491,10 @@ export interface IContextualMenuItem {
inactive?: boolean;
}

export interface IContextualMenuSection extends React.Props<ContextualMenu> {
/**
* React.Props is deprecated and we're removing it in 6.0. Usage of 'any' should go away with it.
*/
export interface IContextualMenuSection extends React.Props<any> {

/**
* The items to include inside the section.
Expand Down