Skip to content

Shimmer: change prop use of subcomponents Line, Circle and Gap.#5739

Merged
Vitalius1 merged 6 commits intomicrosoft:masterfrom
Vitalius1:v-vibr/ShimmerPropDeprecation
Jul 31, 2018
Merged

Shimmer: change prop use of subcomponents Line, Circle and Gap.#5739
Vitalius1 merged 6 commits intomicrosoft:masterfrom
Vitalius1:v-vibr/ShimmerPropDeprecation

Conversation

@Vitalius1
Copy link
Contributor

@Vitalius1 Vitalius1 commented Jul 31, 2018

Pull request checklist

  • Include a change request file using $ npm run change

Description of changes

Removing use of borderStyle prop of Shimmer subcomponents in favor of using mergeStyles API.

Microsoft Reviewers: Open in CodeFlow

@cliffkoh
Copy link
Contributor

borderStyle?: IRawStyle;

It would be great to document all of these interfaces


Refers to: packages/office-ui-fabric-react/src/components/Shimmer/ShimmerCircle/ShimmerCircle.types.ts:42 in 0f11abf. [](commit_id = 0f11abf, deletion_comment = False)

@@ -11,12 +11,11 @@ export function getStyles(props: IShimmerGapStyleProps): IShimmerGapStyles {
const { palette } = theme;
const classNames = getGlobalClassNames(GlobalClassNames, theme);
Copy link
Contributor

Choose a reason for hiding this comment

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

classNames [](start = 8, length = 10)

nit: rename these to globalClassNames so it is clearer e.g. in line 18

@cliffkoh
Copy link
Contributor

borderStyle?: IRawStyle;

these would be also deprecated right?


Refers to: packages/office-ui-fabric-react/src/components/Shimmer/ShimmerGap/ShimmerGap.types.ts:48 in 0f11abf. [](commit_id = 0f11abf, deletion_comment = False)

Copy link
Contributor

@cliffkoh cliffkoh left a comment

Choose a reason for hiding this comment

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

Superficially, looks good but I have a few nits/suggestions for improvement.

@cliffkoh cliffkoh self-assigned this Jul 31, 2018
@cliffkoh
Copy link
Contributor

👍 👍 Thanks for making the changes suggested.

@Vitalius1 Vitalius1 merged commit ed91a17 into microsoft:master Jul 31, 2018
nulikartik pushed a commit to nulikartik/office-ui-fabric-react that referenced this pull request Aug 1, 2018
…osoft#5739)

* Adds deprecation message and renames constant 'styles' to 'borderStyles'.

* Change the use of 'borderStyle' prop on shimmer subcomponents in favor of 'styles' to leverage the 'mergerStyles' API.

* Change log.

* Remove unnecessary style.

* Rename a constant in styles of Shimmer subcomponents for improved semantics.

* Adds documentation to subcomponents  interfaces.
Vitalius1 added a commit to Vitalius1/office-ui-fabric-react that referenced this pull request Aug 9, 2018
dzearing pushed a commit that referenced this pull request Aug 10, 2018
* Cherry pick PR #5067.

* Resolve the conflict.

* Revert all prettier formatting and rename styles to getStyles.

* Cherry-pick PR #5739.

* Fix function returning styles.

* Change file extension.

* Remove change logs.

* Change log for OUFR.

* Change output for fabric-website.

* Fix lint error.
@Vitalius1 Vitalius1 deleted the v-vibr/ShimmerPropDeprecation branch November 7, 2018 21:02
@microsoft microsoft locked as resolved and limited conversation to collaborators Aug 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants