Skip to content

Conversation

AndrewMusgrave
Copy link
Member

WHY are these changes introduced?

Part of #1484

WHAT is this pull request doing?

I'm converting Navigation.Item to a functional component and removing withContext

How to 🎩

Tests / percy / development server

@BPScott BPScott temporarily deployed to polaris-react-pr-1502 May 15, 2019 21:22 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1502 May 15, 2019 21:26 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-1502 May 15, 2019 21:30 Inactive
@AndrewMusgrave AndrewMusgrave added the 🤖Skip Changelog Causes CI to ignore changelog update check. label May 15, 2019
Copy link
Member

@alex-page alex-page left a comment

Choose a reason for hiding this comment

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

There are some warnings from deprecation, not sure if that should be happening:

warn: ["Deprecation: <Navigation.UserMenu /> is deprecated and will be removed in the next major version. Use <TopBar.UserMenu /> instead."]
warn: ["Deprecation: the `userMenu` prop is deprecated and will be removed in the next major version. Pass a user menu into <TopBar /> instead."]

The functionality is working in chrome. Nice work @AndrewMusgrave

@BPScott BPScott requested a deployment to polaris-react-pr-1502 May 28, 2019 18:52 Abandoned
@AndrewMusgrave
Copy link
Member Author

AndrewMusgrave commented May 28, 2019

There are some warnings from deprecation, not sure if that should be happening:

It's all good, they haven't been removed yet 😄

@AndrewMusgrave AndrewMusgrave merged commit 5f03863 into version-4.0.0 May 28, 2019
@AndrewMusgrave AndrewMusgrave deleted the ni-dep-hoc branch May 28, 2019 18:59
@BPScott BPScott changed the title [v4][Navigation.Item] Remove withRef [v4][Navigation.Item] Remove withContext May 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🤖Skip Changelog Causes CI to ignore changelog update check.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants