Skip to content

Conversation

@MLoughry
Copy link
Contributor

Pull request checklist

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

Description of changes

The change to use React portals is breaking key focus scenarios for both OWA and WAC. Backing this out until such issues can be more thoroughly investigated.

Focus areas to test

(optional)

"changes": [
{
"packageName": "office-ui-fabric-react",
"comment": "Revert change to use React.createPortal, until focus leaking issues can be resolved",
Copy link
Contributor

Choose a reason for hiding this comment

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

generalize this comment to be "... until event leaking issues can be resolved"

Copy link
Member

@micahgodbolt micahgodbolt left a comment

Choose a reason for hiding this comment

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

once CI passes

const portalSupport: boolean = !!ReactDOM.createPortal;

export const Layer = styled<ILayerProps, ILayerStyleProps, ILayerStyles>(
portalSupport ? PortalLayerBase : LayerBase,
Copy link
Contributor

Choose a reason for hiding this comment

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

Just set this flag to false for now?

Copy link
Contributor

Choose a reason for hiding this comment

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

If we want to create a release without the change, that would be the least invasive change I feel and would make it easier for forward fix the components that might have changed behavior.

You should be able to validate with a conditional breakpoint that changes this var

@jspurlin jspurlin merged commit f6bc8fc into microsoft:master May 11, 2018
Markionium added a commit to Markionium/office-ui-fabric-react that referenced this pull request May 13, 2018
…i-fabric-react into parallel-tsc

* 'parallel-tsc' of https://github.com/Markionium/office-ui-fabric-react:
  Variants: have project use OUFR instead of just styling (microsoft#4854)
  Add more customization hooks to ProgressIndicator (microsoft#4566)
  Issue#4832: Break BaseButton Types hard dependency from class ContextualMenu (microsoft#4845)
  Applying package updates.
  Revert react portals change (microsoft#4840)
  Update ImageOverview.md
  Fixes duplicate reading of suggestions on people picker (microsoft#4765)
  Persona: Deprecate primaryText (microsoft#4811)
  Experiments: Fix Fluent theme color names (microsoft#4834)
  Applying package updates.
  Add JasonGore to command bar codeowners
  Fix index import (microsoft#4826)
  Added overflowMenuProps property to CommandBar (microsoft#4818)
  Fluent theme: Fix imports to use relative paths (microsoft#4831)
  ContextualMenuItem: adding secondaryText (microsoft#4788)
  ComboBox: Option Performance Optimization (microsoft#4782)
Markionium added a commit to Markionium/office-ui-fabric-react that referenced this pull request May 13, 2018
* master: (95 commits)
  Variants: have project use OUFR instead of just styling (microsoft#4854)
  Add more customization hooks to ProgressIndicator (microsoft#4566)
  Issue#4832: Break BaseButton Types hard dependency from class ContextualMenu (microsoft#4845)
  Applying package updates.
  Revert react portals change (microsoft#4840)
  Update ImageOverview.md
  Fixes duplicate reading of suggestions on people picker (microsoft#4765)
  Persona: Deprecate primaryText (microsoft#4811)
  Experiments: Fix Fluent theme color names (microsoft#4834)
  Applying package updates.
  Add JasonGore to command bar codeowners
  Fix index import (microsoft#4826)
  Added overflowMenuProps property to CommandBar (microsoft#4818)
  Fluent theme: Fix imports to use relative paths (microsoft#4831)
  ContextualMenuItem: adding secondaryText (microsoft#4788)
  ComboBox: Option Performance Optimization (microsoft#4782)
  Marqueeselection style update (microsoft#4803)
  Applying package updates.
  FocusZone: Add the ability to stop focus from propagating outside the FocusZone (microsoft#4823)
  Unknown persona coin (microsoft#4809)
  ...
@microsoft microsoft locked as resolved and limited conversation to collaborators Aug 30, 2019
@MLoughry MLoughry deleted the revert-react-portals branch April 22, 2020 20:42
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.

8 participants