-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Layer SCSS to MergeStyles #3979
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.
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
Changes from 5 commits
fa130ec
58531b6
848f05c
0865bf8
12b8d61
1a4b677
a7b39b3
dbc609a
0be6aee
657117b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| { | ||
| "changes": [ | ||
| { | ||
| "packageName": "office-ui-fabric-react", | ||
| "comment": "Convert Layer component to mergeStyles", | ||
| "type": "minor" | ||
| } | ||
| ], | ||
| "packageName": "office-ui-fabric-react", | ||
| "email": "v-brgarl@microsoft.com" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,181 @@ | ||
| /* tslint:disable:no-unused-variable */ | ||
| import * as React from 'react'; | ||
| import * as ReactDOM from 'react-dom'; | ||
| /* tslint:enable:no-unused-variable */ | ||
|
|
||
| import { Fabric } from '../../Fabric'; | ||
| import { | ||
| ILayerProps, | ||
| ILayerStyleProps, | ||
| ILayerStyles, | ||
| } from './Layer.types'; | ||
| import { | ||
| css, | ||
| BaseComponent, | ||
| classNamesFunction, | ||
| customizable, | ||
| getDocument, | ||
| setVirtualParent | ||
| } from '../../Utilities'; | ||
| // import * as stylesImport from './Layer.scss'; | ||
| // const styles: any = stylesImport; | ||
|
|
||
| let _layersByHostId: { [hostId: string]: LayerBase[] } = {}; | ||
| let _defaultHostSelector: string | undefined; | ||
|
|
||
| const getClassNames = classNamesFunction<ILayerStyleProps, ILayerStyles>(); | ||
|
|
||
| // @customizable('Layer', ['theme']) | ||
| export class LayerBase extends BaseComponent<ILayerProps, {}> { | ||
|
|
||
| public static defaultProps: ILayerProps = { | ||
| onLayerDidMount: () => undefined, | ||
| onLayerWillUnmount: () => undefined | ||
| }; | ||
|
|
||
| private _rootElement: HTMLElement; | ||
| private _host: Node; | ||
| private _layerElement: HTMLElement | undefined; | ||
| private _hasMounted: boolean; | ||
| /** | ||
| * Used for notifying applicable Layers that a host is available/unavailable and to re-evaluate Layers that | ||
| * care about the specific host. | ||
| */ | ||
| public static notifyHostChanged(id: string) { | ||
| if (_layersByHostId[id]) { | ||
| _layersByHostId[id].forEach(layer => layer.forceUpdate()); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Sets the default target selector to use when determining the host in which | ||
| * Layered content will be injected into. If not provided, an element will be | ||
| * created at the end of the document body. | ||
| * | ||
| * Passing in a falsey value will clear the default target and reset back to | ||
| * using a created element at the end of document body. | ||
| */ | ||
| public static setDefaultTarget(selector?: string) { | ||
| _defaultHostSelector = selector; | ||
| } | ||
|
|
||
| constructor(props: ILayerProps) { | ||
| super(props); | ||
|
|
||
| this._warnDeprecations({ | ||
| onLayerMounted: 'onLayerDidMount' | ||
| }); | ||
|
|
||
| if (this.props.hostId) { | ||
| if (!_layersByHostId[this.props.hostId]) { | ||
| _layersByHostId[this.props.hostId] = []; | ||
| } | ||
|
|
||
| _layersByHostId[this.props.hostId].push(this); | ||
| } | ||
| } | ||
|
|
||
| public componentDidMount() { | ||
| this.componentDidUpdate(); | ||
| } | ||
|
|
||
| public componentWillUnmount() { | ||
| this._removeLayerElement(); | ||
|
|
||
| if (this.props.hostId) { | ||
| _layersByHostId[this.props.hostId] = _layersByHostId[this.props.hostId].filter(layer => layer !== this); | ||
| if (!_layersByHostId[this.props.hostId].length) { | ||
| delete _layersByHostId[this.props.hostId]; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| public componentDidUpdate() { | ||
| let host = this._getHost(); | ||
|
|
||
| const { getStyles, theme } = this.props; | ||
| const classNames = getClassNames(getStyles!, | ||
| { | ||
| theme: theme!, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| isNotHost: !this.props.hostId | ||
| } | ||
| ); | ||
|
|
||
| if (host !== this._host) { | ||
| this._removeLayerElement(); | ||
| } | ||
|
|
||
| if (host) { | ||
| this._host = host; | ||
|
|
||
| if (!this._layerElement) { | ||
| let doc = getDocument(this._rootElement) as Document; | ||
|
|
||
| this._layerElement = doc.createElement('div'); | ||
| // this._layerElement.className = css('ms-Layer', { ['ms-Layer--fixed ' + styles.rootIsFixed]: !this.props.hostId }); | ||
| this._layerElement.className = classNames.root; | ||
|
|
||
| host.appendChild(this._layerElement); | ||
| setVirtualParent(this._layerElement, this._rootElement); | ||
| } | ||
|
|
||
| // Using this 'unstable' method allows us to retain the React context across the layer projection. | ||
| ReactDOM.unstable_renderSubtreeIntoContainer( | ||
| this, | ||
| ( | ||
| // <Fabric className={ css('ms-Layer-content', styles.content) }> | ||
| <Fabric className={ classNames.content }> | ||
| { this.props.children } | ||
| </Fabric> | ||
| ), | ||
| this._layerElement, | ||
| () => { | ||
| if (!this._hasMounted) { | ||
| this._hasMounted = true; | ||
|
|
||
| // TODO: @deprecated cleanup required. | ||
| if (this.props.onLayerMounted) { | ||
| this.props.onLayerMounted(); | ||
| } | ||
|
|
||
| this.props.onLayerDidMount!(); | ||
| } | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| public render() { | ||
| return ( | ||
| <span | ||
| className='ms-Layer' | ||
| ref={ this._resolveRef('_rootElement') } | ||
| /> | ||
| ); | ||
| } | ||
|
|
||
| private _removeLayerElement() { | ||
| if (this._layerElement) { | ||
| this.props.onLayerWillUnmount!(); | ||
|
|
||
| ReactDOM.unmountComponentAtNode(this._layerElement); | ||
| let parentNode = this._layerElement.parentNode; | ||
| if (parentNode) { | ||
| parentNode.removeChild(this._layerElement); | ||
| } | ||
| this._layerElement = undefined; | ||
| this._hasMounted = false; | ||
| } | ||
| } | ||
|
|
||
| private _getHost(): Node { | ||
| let { hostId } = this.props; | ||
| let doc = getDocument(this._rootElement) as Document; | ||
|
|
||
| if (hostId) { | ||
| return doc.getElementById(hostId) as Node; | ||
| } else { | ||
| return _defaultHostSelector ? doc.querySelector(_defaultHostSelector) as Node : doc.body; | ||
| } | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| import { ILayerStyleProps, ILayerStyles } from './Layer.types'; | ||
| import { | ||
| IStyle, | ||
| ITheme, | ||
| } from '../../Styling'; | ||
|
|
||
| export const getStyles = ( | ||
| props: ILayerStyleProps | ||
| ): ILayerStyles => { | ||
| const { | ||
| className, | ||
| theme, | ||
| isNotHost | ||
| } = props; | ||
|
|
||
| // const { palette, semanticColors } = theme; | ||
|
|
||
| return ({ | ||
| root: [ | ||
| 'ms-Layer', | ||
| isNotHost && 'ms-Layer--fixed' && { | ||
| position: 'fixed', | ||
| zIndex: 1000000, | ||
| top: 0, | ||
| left: 0, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Left get changed to right automatically by getClassNames when the page is RTL. I don't believe this should impact anything but please just verify that it works in RTL.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks fine in RTL mainly because the width is '100vw', so left:0 and right:0 are the same either way. The text automatically aligns to the right tho, but that's not part of this style set. |
||
| width: '100vw', | ||
| height: '100vh', | ||
| visibility: 'hidden' | ||
| } | ||
| ], | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. missing className at the end here.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we have a snapshot test, it would be great to update it and pass a |
||
| content: [ | ||
| 'ms-Layer-content', | ||
| { | ||
| visibility: 'visible' | ||
| } | ||
| ] | ||
| }); | ||
| }; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to uncomment this before checkin, maybe there is something we can do here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see the old one didn't have it. I'm ok omitting it for now if we can't figure it out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is commented out due to a bug, #3988. I'm leaving this commented out so it can be turned on later easily, but it's not blocking for this specific component because at least currently it does not need to use the theme.