Shimmer: refactor in preparation for migration to OUFR by Vitalius1 · Pull Request #4958 · microsoft/fluentui

Vitalius1 · 2018-05-23T02:13:51Z

Pull request checklist

Addresses an existing issue: Fixes #0000
Include a change request file using $ npm run change

Description of changes

This PR is the last refactor of Shimmer and this version is intended for migration to OUFR. Most of the changes are moving stuff around so functionally nothing changed.

Shimmer - Example. (give it a couple seconds to load )

Things done:

Support for RTL
Support for HighContrast
Support for screen-readers
Support for theming
Built following mergeStyles pattern
Modularized as much as possible
Support for different browser (IE11, Edge, Firefox, Chrome, Safari, Opera)
A lot of examples on using Shimmer for building various skeletons of UI
Deprecated props are warned in the console

Microsoft Reviewers: Open in CodeFlow

Removes some styling from shimmer container.

Setup of elements wrapper.

Extracts some logic to a new module.

Still a lot to do.

…lors.

…the transition animation is done.

Refactors common styles in ShimmerLine.styles

High Contrast capability.

jordandrako · 2018-05-23T18:12:29Z

packages/experiments/src/components/Shimmer/Shimmer.base.tsx


  public render(): JSX.Element {
    const {
      getStyles,


Approved, but the getStyles prop name is being replaced with styles in 6.0. Depending on when you want this merged, before or after 6.0 is merged to master, you may need to replace getStyles in the props with styles. The function getStyles within the .styles.ts file can remain getStyles.

See this commit for details: f226fb4

Vitalius1 · 2018-05-24T17:28:06Z

@dzearing Do you know why bundlesize check is still pending? Also, is there anyone from your team available for a code review? Thanks!

atneik · 2018-05-24T21:21:19Z

packages/experiments/src/components/Shimmer/ShimmerElementsGroup/ShimmerElementsGroup.types.ts

+   * Optional boolean for enabling flexWrap of the container containing the shimmerElements.
+   * @default false
+   */
+  flexWrap?: boolean;


Should this be true by default?

No. This subcomponent is also used internally by ShimmerBase when everything stays in one line. So by default is false.

atneik

JasonGore · 2018-05-25T23:52:23Z

packages/experiments/src/components/Shimmer/Shimmer.base.tsx

+    this._warnMutuallyExclusive({
+      'lineElements': 'shimmerElements',
+      'customElementsGroup': 'lineElements'
+    });


There's no need to support deprecated props in an experimental component. Of course anything using it will have to be changed and tested.

@JasonGore This experimental component is used in odsp-common repo in items-view and odsp-shared-react packages. So removing the deprecated props will cause a break on their side. The intention is to merge this change and right after I will migrate this component to OUFR without the deprecated props. Next step will be to go and change in odsp-common the imports from experiments to office-ui-fabric-react and use the correct props. Only when all this happens I will be able to remove the deprecated props in experiments package without breaking the contract with Shimmer API.

Ok, thanks for the explanation

JasonGore · 2018-05-26T00:32:41Z

packages/experiments/src/components/Shimmer/Shimmer.types.ts

+   * @deprecated Use 'top' instead
+   */
+  TOP = 3,
+


No need to support deprecated values

same as above

JasonGore · 2018-05-26T00:34:17Z

packages/experiments/src/components/Shimmer/Shimmer.types.ts

+   * @default false
+   * @deprecated Use 'customElementsGroup' instead.
+   */
+  isBaseStyle?: boolean;


Please remove deprecated props

same as above

Justification provided

* master: Applying package updates. Shimmer: refactor in preparation for migration to OUFR (microsoft#4958) Applying package updates. Theming: fix error colors (microsoft#4969) MaskedTextField: Added event callback passthrough (microsoft#4956) Experiments/Nav component: Enable auto expand until the next manual expand disables the auto expand (microsoft#4996) Applying package updates. Experiments/Nav component: Auto select/expand based on the selectedKey prop (microsoft#4984) StickyPane: fix Array.from in Ie (microsoft#4982) ContextualMenu has aria-labelledBy referencing element not in DOM (microsoft#4963) Keyboard support for the slim version of experiments/Nav component and added aria attributes (microsoft#4981) Move ghdocs to wiki (microsoft#4977) Remove build artifacts (microsoft#4976) Revisited the Multi-select Combo box initial state selection fix (microsoft#4884) Applying package updates. readme cleanup (microsoft#4972) Theming: add the bodyBackgroundDarker semantic slot (microsoft#4957) Improvements for auto-select opt-in mode (microsoft#4914)

* master: (85 commits) Applying package updates. Shimmer: refactor in preparation for migration to OUFR (microsoft#4958) Applying package updates. Theming: fix error colors (microsoft#4969) MaskedTextField: Added event callback passthrough (microsoft#4956) Experiments/Nav component: Enable auto expand until the next manual expand disables the auto expand (microsoft#4996) Applying package updates. Experiments/Nav component: Auto select/expand based on the selectedKey prop (microsoft#4984) StickyPane: fix Array.from in Ie (microsoft#4982) ContextualMenu has aria-labelledBy referencing element not in DOM (microsoft#4963) Keyboard support for the slim version of experiments/Nav component and added aria attributes (microsoft#4981) Move ghdocs to wiki (microsoft#4977) Remove build artifacts (microsoft#4976) Revisited the Multi-select Combo box initial state selection fix (microsoft#4884) Applying package updates. readme cleanup (microsoft#4972) Theming: add the bodyBackgroundDarker semantic slot (microsoft#4957) Improvements for auto-select opt-in mode (microsoft#4914) Applying package updates. Revert ChoiceGroup change in 5.0 to minimize potential partner impact. (microsoft#4962) ...

This reverts commit 5ecdca8.

Vitalie Braga added 30 commits May 8, 2018 22:55

Adds new prop: className.

6d0decd

Removes some styling from shimmer container.

Removes unnecessary styles.

94ea1cd

Setup of elements wrapper.

Refactoring some styles.

0db37ed

Extracts some logic to a new module.

A lot of refactor.

762e3c5

Still a lot to do.

Change new prop name for better semantics.

1c409ba

Apply new prop to application in detailsList example.

f2d84d5

Use the new prop in TilesList.

053648b

More REFACTOR.

567983c

More examples...

8c375a6

Enums refactor and documentation.

3b2d554

Overflow text fix in LoadData example.

ce8d116

Moves deprecated props down.

8689193

Brings logic for RTL animation.

df48bbb

Introduces @Customizable decorator to enable custom theming of the co…

a536e6c

…lors.

Sets an example for getStyles prop.

22a23e0

Adds more documentation to Shimmer Page.

c2af0ab

Sets up a flag to remove the shimmerWrapper from the flow as soon as …

e0831cf

…the transition animation is done.

Adds rounded corners to shimmer lines.

9607b8f

More refactor :)

61356d3

Clears a timeout for application example.

a063dd2

Adds accessibility capabilities for screenreaders.

4a261d1

Refactors some highContrast styling.

dff7fd9

Refactors common styles in ShimmerLine.styles

Modularize ShimmerElementsGroup

074a924

Customize ShimmerElementsGroup.

6fcd497

Refactor mergeStyles pattern in ShimmerElementsGroup

c12bad1

Convert subelements to the new pattern of mergeStyles.

7e68d54

Removes some styles.

535bd69

Fixes support for various browsers.

ed177be

High Contrast capability.

Fill color for line corners in high contrast mode.

8ab9d75

Rush change log.

d8ae65b

Vitalius1 requested review from Jahnp, atneik, dzearing, jordandrako and lynamemi May 23, 2018 02:13

Fix tslint warnings.

f2a6c76

jordandrako approved these changes May 23, 2018

View reviewed changes

jordandrako reviewed May 23, 2018

View reviewed changes

atneik reviewed May 24, 2018

View reviewed changes

atneik approved these changes May 24, 2018

View reviewed changes

Adds a default prop in ShimmerElementsGroup

84fe6c8

JasonGore previously requested changes May 26, 2018

View reviewed changes

Vitalius1 merged commit 5ecdca8 into microsoft:master May 26, 2018

cliffkoh added the 6.0-follow-up-required label May 30, 2018

cliffkoh added a commit that referenced this pull request May 30, 2018

Revert "Shimmer: refactor in preparation for migration to OUFR (#4958)"

588fe96

This reverts commit 5ecdca8.

Vitalius1 mentioned this pull request May 30, 2018

Shimmer: resolve conflicts #5034

Merged

2 tasks

Vitalius1 deleted the v-vibr/RefactorShimmer branch July 3, 2018 00:04

microsoft locked as resolved and limited conversation to collaborators Aug 30, 2019

Conversation

Vitalius1 commented May 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request checklist

Description of changes

Microsoft Reviewers: Open in CodeFlow

Uh oh!

jordandrako May 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Vitalius1 commented May 24, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

atneik left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Vitalius1 commented May 23, 2018 •

edited

Loading

jordandrako May 23, 2018 •

edited

Loading