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.

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

ResizeGroup SCSS to MergeStyles Part 2: Style Conversion #4072

Merged
merged 17 commits into from
Mar 1, 2018
Merged

ResizeGroup SCSS to MergeStyles Part 2: Style Conversion #4072

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
6766f11
convert scss to mergeStyles
oengusmacinog-zz Feb 22, 2018
2320f81
rush change
oengusmacinog-zz Feb 22, 2018
1c3a3a8
updated snapshots
oengusmacinog-zz Feb 22, 2018
62109bf
update experiments snapshots
oengusmacinog-zz Feb 22, 2018
2438eb4
temp fix for shallow test with decorator usage
oengusmacinog-zz Feb 22, 2018
4944214
added shallowUntilTarget function to Utilities for use with decorated…
oengusmacinog-zz Feb 23, 2018
e7825b0
cleanup
oengusmacinog-zz Feb 23, 2018
d3cfa7b
moved shallowUntilTarget into fabrics common folder
oengusmacinog-zz Feb 23, 2018
6559ee6
clean up imports
oengusmacinog-zz Feb 23, 2018
1e8dd14
remove scss
oengusmacinog-zz Feb 27, 2018
125adcd
added base to index
oengusmacinog-zz Feb 27, 2018
a0e687d
shallowUntilTarget default shallow functionality
oengusmacinog-zz Feb 27, 2018
0779d03
removing base from index so styles are not optional
oengusmacinog-zz Feb 27, 2018
912cf74
moved measured styles to inline
oengusmacinog-zz Feb 28, 2018
5b6268f
updated snapshots
oengusmacinog-zz Feb 28, 2018
186ed05
missing semicolon
oengusmacinog-zz Feb 28, 2018
b7a8756
updated experiments snapshots
oengusmacinog-zz Feb 28, 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/resizegroup-scss2ms-pt2_2018-02-22-21-19.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": "Converting ResizeGroup SCSS to MergeStyles step 2 - style converstion",
"type": "minor"
}
],
"packageName": "office-ui-fabric-react",
"email": "[email protected]"
}
15 changes: 13 additions & 2 deletions packages/experiments/src/components/CommandBar/__snapshots__/CommandBar.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,21 @@

exports[`CommandBar renders commands correctly 1`] = `
<div
className="ms-ResizeGroup TestClassName"
className=
ms-ResizeGroup
TestClassName
{
display: block;
position: relative;
}
>
<div
className=""
className=
ms-ResizeGroup--measured
{
position: fixed;
visibility: hidden;
}
>
<div
className=
Expand Down
64 changes: 64 additions & 0 deletions packages/office-ui-fabric-react/src/common/shallowUntilTarget.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import { shallow, ShallowWrapper } from 'enzyme';

/**
* Duplicated enzyme's ShallowRendererProps
*
* @internal
*/
export interface IShallowRendererProps {
lifecycleExperimental?: boolean;
disableLifecycleMethods?: boolean;
}

/**
* ShallowUntilTarget Interface
*
* @internal
*/
export interface IShallowUntilTarget {
maxTries: number;
shallowOptions: IShallowRendererProps;
}

/**
* An extention of enzyme's shallow function which will fail to work
* with decorated components and/or components using the styled() function.
* This function allows you to pass a 'target' component (e.g. ComponentBase)
* and keep running shallow on each child component till a match is found.
*
* @public
*/
export function shallowUntilTarget<P, S>(
componentInstance: React.ReactElement<P>,
TargetComponent: string,
Copy link
Contributor

Choose a reason for hiding this comment

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

You could have target component be type of Function, and then users could pass in the actual class that they care about. You could then access the string by calling TargetComponent.name and it would work out fine. That way it keeps things a bit more type safe since they would need to import the actual class they care about. This is just a suggestion, if you feel like trying it out that would be cool, otherwise no worries.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was the original way that I had implemented this, but specifically for decorated functions like ResizeGroupBase, the name becomes 'ComponentWithInjectedProps' so it returns before it should. Passing a string name into the function was a way to avoid this.

options: IShallowUntilTarget = {
maxTries: 10,
shallowOptions: {}
}
): ShallowWrapper {
const { maxTries, shallowOptions } = options;

let root = shallow<P, S>(componentInstance, shallowOptions);

if (typeof root.type() === 'string') {
// If type() is a string then it's a DOM Node.
// If it were wrapped, it would be a React component.
throw new Error('Cannot unwrap this component because it is not wrapped');
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that we shouldn't throw in this case. I should be able to use this function anywhere that shallow can be used. If the TargetComponent is a div, and the root is a div then that's a fine case. If this should only be used in the case of a decorated component, then the name should reflect that. Something like shallowDecoratedComponent

}

for (let tries = 1; tries <= maxTries; tries++) {
// Check for target as a string to avoid conflicts
// with decoratored components name
if (root.type().toString().includes(TargetComponent)) {
// Now that we found the target component, render it.
return root.first().shallow(shallowOptions);
}
// Unwrap the next component in the hierarchy.
root = root.first().shallow(shallowOptions);
Copy link
Contributor

Choose a reason for hiding this comment

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

While I don't think that any decorators we currently have render multiple siblings, it is possible that this will miss a component that isn't the first child.

}

throw new Error(
`Could not find ${TargetComponent} in React instance: ${componentInstance};
gave up after ${maxTries} tries`
);
}
28 changes: 24 additions & 4 deletions ...s/office-ui-fabric-react/src/components/Breadcrumb/__snapshots__/Breadcrumb.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,20 @@

exports[`Breadcrumb renders breadcumb correctly 1`] = `
<div
className="ms-ResizeGroup"
className=
ms-ResizeGroup
{
display: block;
position: relative;
}
>
<div
className=""
className=
ms-ResizeGroup--measured
{
position: fixed;
visibility: hidden;
}
>
<div
aria-label={undefined}
Expand Down Expand Up @@ -146,10 +156,20 @@ exports[`Breadcrumb renders breadcumb correctly 1`] = `

exports[`Breadcrumb renders breadcumb correctly 2`] = `
<div
className="ms-ResizeGroup"
className=
ms-ResizeGroup
{
display: block;
position: relative;
}
>
<div
className=""
className=
ms-ResizeGroup--measured
{
position: fixed;
visibility: hidden;
}
>
<div
aria-label={undefined}
Expand Down
18 changes: 14 additions & 4 deletions packages/office-ui-fabric-react/src/components/ResizeGroup/ResizeGroup.base.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,18 @@ import * as React from 'react';
import * as PropTypes from 'prop-types';
import {
BaseComponent,
classNamesFunction,
css,
customizable,
divProperties,
getNativeProps,
provideContext
} from '../../Utilities';
import { IResizeGroupProps } from './ResizeGroup.types';
import {
IResizeGroupProps,
IResizeGroupStyles,
IResizeGroupStyleProps
} from './ResizeGroup.types';
import * as styles from './ResizeGroup.scss';

const RESIZE_DELAY = 16;
Expand Down Expand Up @@ -284,6 +290,9 @@ const MeasuredContext = provideContext({
return { isMeasured: true };
});

const getClassNames = classNamesFunction<IResizeGroupStyleProps, IResizeGroupStyles>();

@customizable('ResizeGroup', ['theme'])
export class ResizeGroupBase extends BaseComponent<IResizeGroupProps, IResizeGroupState> {
private _nextResizeGroupStateProvider = getNextResizeGroupStateProvider();
private _root: HTMLElement;
Expand All @@ -299,14 +308,15 @@ export class ResizeGroupBase extends BaseComponent<IResizeGroupProps, IResizeGro
}

public render() {
const { onRenderData, className } = this.props;
const { onRenderData, className, getStyles, theme } = this.props;
const { dataToMeasure, renderedData } = this.state;
const divProps = getNativeProps(this.props, divProperties, ['data']);
const classNames = getClassNames(getStyles!, { theme: theme!, className });

return (
<div { ...divProps } className={ css('ms-ResizeGroup', className) } ref={ this._resolveRef('_root') }>
<div { ...divProps } className={ classNames.root } ref={ this._resolveRef('_root') }>
{ this._nextResizeGroupStateProvider.shouldRenderDataToMeasureInHiddenDiv(dataToMeasure) && (
<div className={ css(styles.measured) } ref={ this._resolveRef('_measured') }>
<div className={ classNames.measured } ref={ this._resolveRef('_measured') }>
<MeasuredContext>{ onRenderData(dataToMeasure) }</MeasuredContext>
</div>
) }
Expand Down
16 changes: 11 additions & 5 deletions packages/office-ui-fabric-react/src/components/ResizeGroup/ResizeGroup.styles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,17 @@ export const getStyles = (
root: [
'ms-ResizeGroup',
{
// Insert css properties

}
display: 'block',
position: 'relative'
},
className
],

// Insert className styles
measured: [
'ms-ResizeGroup--measured',
{
position: 'fixed',
visibility: 'hidden'
}
]
});
};
10 changes: 5 additions & 5 deletions packages/office-ui-fabric-react/src/components/ResizeGroup/ResizeGroup.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import * as React from 'react';
import { shallow } from 'enzyme';
import { shallowUntilTarget } from '../../common/shallowUntilTarget';

import { ResizeGroup } from './ResizeGroup';
import { ResizeGroupBase, IResizeGroupState, getNextResizeGroupStateProvider, getMeasurementCache } from './ResizeGroup.base';
import { IResizeGroupState, getNextResizeGroupStateProvider, getMeasurementCache } from './ResizeGroup.base';
import { IResizeGroupProps } from './ResizeGroup.types';
import * as sinon from 'sinon';
import * as renderer from 'react-test-renderer';
Expand Down Expand Up @@ -45,13 +45,13 @@ describe('ResizeGroup', () => {
const renderedDataId = 'onRenderDataId';
const onRenderData = (data: any) => <div id={ renderedDataId }> Rendered data: { data.content }</div >;

const wrapper = shallow<IResizeGroupProps, IResizeGroupState>(
<ResizeGroupBase
const wrapper = shallowUntilTarget<IResizeGroupProps, IResizeGroupState>(
<ResizeGroup
data={ initialData }
onReduceData={ onReduceScalingData }
onRenderData={ onRenderData }
/>
);
, 'ResizeGroupBase');

expect(wrapper.containsMatchingElement(onRenderData(initialData))).toEqual(true);
});
Expand Down
5 changes: 5 additions & 0 deletions packages/office-ui-fabric-react/src/components/ResizeGroup/ResizeGroup.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,4 +87,9 @@ export interface IResizeGroupStyles {
* Style for the root element.
*/
root: IStyle;

/**
* Style set for the measured element.
*/
measured: IStyle;
}
15 changes: 13 additions & 2 deletions ...office-ui-fabric-react/src/components/ResizeGroup/__snapshots__/ResizeGroup.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,21 @@

exports[`ResizeGroup renders the ResizeGroup correctly 1`] = `
<div
className="ms-ResizeGroup TestClassName"
className=
ms-ResizeGroup
TestClassName
{
display: block;
position: relative;
}
>
<div
className=""
className=
ms-ResizeGroup--measured
{
position: fixed;
visibility: hidden;
}
>
<div
id="onRenderDataId"
Expand Down