Pickers: convert to CSS-in-JS Part 1

[Vitalius1]

Pull request checklist

• Addresses an existing issue: Pickers: Convert to css-in-js #5411
• Include a change request file using $ npm run change

Description of changes

This PR brings the first part of the pickers' conversion to use CSS-in-JS. Things that were done in this part:

1. Add BasePicker.styles.ts to provide CSS-in-JS styling for the already existing pickers (TagPicker, PeoplePicker) and still leave support for Sass styles for custom pickers implemented by extending from BasePicker class without injecting the default styles.

2. Added comments on explaining why the above is needed.

3. Add styles and theme optional props to IBasePickerProps interface.

4. Changed tests in BasePicker.test.ts to use correctly a public method on the component instance by accessing its componentRef (this surfaced after wrapping the pickers with the styled function and the linter giving me red lines).

5. Removing this parameter in the PeoplePicker's createGenericItem default prop added by Initials for phone numbers #4376
   https://github.com/OfficeDev/office-ui-fabric-react/blob/31783e67b172c645d15e169b54c84e8bfed5d6ca/packages/office-ui-fabric-react/src/components/pickers/PeoplePicker/PeoplePicker.tsx#L64
   It was never actually working and for some reason, this was not caught by types because this prop is only allowing to pass 2 arguments:
   https://github.com/OfficeDev/office-ui-fabric-react/blob/31783e67b172c645d15e169b54c84e8bfed5d6ca/packages/office-ui-fabric-react/src/components/pickers/BasePicker.types.ts#L120
   ... and later is called only with those 2 arguments in here:
   https://github.com/OfficeDev/office-ui-fabric-react/blob/31783e67b172c645d15e169b54c84e8bfed5d6ca/packages/office-ui-fabric-react/src/components/pickers/BasePicker.tsx#L795
   After wrapping the Base classes with the styled function I started to get type errors. So I discussed it with @joschect and I consider it's safe to be removed. If later this functionality needs to be added (it was never actually working in the first place) this should be done in a separate PR with more changes than the initial mentioned PR introduced. @cliffkoh would appreciate your input with this surface API change as this function was exported even though it's just a default prop provided by PeoplePicker. Even if someone did import to use it I don't see a way to pass that argument in it.

6. A little cleanup of the TagPicker example by moving a state interface into the same file as the example for user visibility and some more type fixes. Plus fixing the same issue as in tests file with the improper use of the public method of the component instance. This was causing the linter to complain that property items does not exist on TagPicker. Changing it to TagPickerBase was solving the errors but user should not care about the Base and use the componentRef prop instead.

7. Updated the API file and snapshots to reflect the current changes

Part 2 PR will consist of converting the picker items (Tags, Personas).

Focus areas to test

Need some feedback on moving the string classNames from being always on the component elements to the styles file, where in case globalClassNames are disabled in the Theme, might break some already existing custom Sass overrides in consumers apps. Should I skip the step of getGlobalClassNames(GlobalClassNames, theme)?

Also, regarding the number 6 in my list above, because we had this example displayed for a long time for users to see, now that accessing those methods is not working that way, we might get into breaking someone's app with this change in case they used this example. Should we still go with this change?

Microsoft Reviewers: Open in CodeFlow

[Vitalius1]

Have no idea what are those screener changes mean. It doesn't look like they changed how they look.

[cliffkoh]

Their values changed, e.g.

[Vitalius1]

But what is box-align? I ported the styles exactly as they were.

https://developer.mozilla.org/en-US/docs/Web/CSS/box-align

[cliffkoh]

As far as I can tell, looks great. I agree with your approach. Thanks for the thoughtfulness here.

Having a fallback to scss is, as far as I can tell, unfortunately necessary primarily for theming purposes if someone were indeed to extend the BasePicker.

Thanks for the very detailed documentation of your approach and for the comments.

[Vitalius1]

Thank you @cliffkoh for reviewing. Would appreciate a little bit more of your time in hearing your thoughts regarding the number 5 in my list of changes and the questions under the Focus areas to test section of the description.

[cliffkoh]

@Vitalius1 I don't see any issue with number 5 :) It seems sound to me.

[KevinTCoughlin]

The majority of the code changes here seem sound, kudos to the thoroughness done here.

Regarding allowPhoneInitials it seems like it may be shared between Persona and Picker code, but I'm having difficulty following. Its my only hesitation to sign-off on this PR.

It might be worth involving the Persona folks to double check.

Otherwise I don't see any breaking API contract changes... Nice approach with maintaining backwards-compat with SCSS for the time-being.

Edit: Read thru the code locally and came to the conclusion that allowPhoneInitials is unused across Picker code.

[KevinTCoughlin]

I reviewed the createGenericItem and allowPhoneInitials removal and I think you are correct that it is never used therefor safe to remove.

👍

[msft-github-bot]

🎉office-ui-fabric-react@v6.113.0 has been released which incorporates this pull request.:tada:

Handy Links:

• Release Notes
• Official NPM Release
• Test the release with Developer Demo Site