Skip to content
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

Update context menu components to React 16.3 lifecycle #887

Merged
merged 4 commits into from
May 30, 2018

Conversation

chandlerprall
Copy link
Contributor

Associated with #763

  • Refactored EuiContextMenu and EuiContextMenuPanel to React 16.3's lifecycle. This moves computed values in both components off of instance members and into React's state.
  • Added 2 more examples to context menu docs
  • Fixed some lurking bugs around allowing dynamic content in the menu panels

@chandlerprall chandlerprall requested a review from nreese May 29, 2018 20:36
@chandlerprall chandlerprall changed the title React 16 3 context menu Update context menu components to React 16.3 lifecycle May 29, 2018
@nreese
Copy link
Contributor

nreese commented May 29, 2018

The first, and the last examples do not open menus in Chrome. Only the caret and border are visible

screen shot 2018-05-29 at 3 40 08 pm

screen shot 2018-05-29 at 3 40 22 pm

@chandlerprall
Copy link
Contributor Author

@nreese have you run yarn in EUI since the upgrade to React@16.3 ? This seems likely to happen if the new getDerivedStateFromProps isn't executing

Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

lgtm

@chandlerprall chandlerprall merged commit d3da542 into elastic:master May 30, 2018
@chandlerprall chandlerprall deleted the react-16-3-context-menu branch May 30, 2018 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants