Skip to content

Conversation

kvendrik
Copy link
Member

@kvendrik kvendrik commented Jan 16, 2019

WHY are these changes introduced?

I'm deprecating Navigation.UserMenu to make room for the new top bar layout in which TopBar.UserMenu will stay visible on mobile (details).

WHAT is this pull request doing?

  • Adds a deprecation notice (console.warn) to Navigation.UserMenu.
  • Adds a deprecation doc comment to Navigation.UserMenu.
  • Adds deprecation rationale to Navigation.UserMenu's documentation.
  • Adds an item to the changelog.

Before shipping: make sure the release in the docs is correct

How to 🎩

1. Copy-paste this code in playground/Playground.tsx:
import * as React from 'react';
import {Navigation} from '../src';

interface State {}

export default class Playground extends React.Component<{}, State> {
  render() {
    return <Navigation.UserMenu avatarInitials="JD" />;
  }
}
2. Make sure a deprecation notice shows up in the console.

Note: shipping this together with the frame layout changes. Version number will need to be updated before merged.

@kvendrik kvendrik requested a review from danrosenthal January 16, 2019 20:34
@BPScott BPScott temporarily deployed to polaris-react-pr-887 January 16, 2019 20:34 Inactive
@kvendrik kvendrik mentioned this pull request Jan 17, 2019
3 tasks
@kvendrik kvendrik requested a review from dleroux January 17, 2019 20:59
Copy link
Contributor

@tmlayton tmlayton left a comment

Choose a reason for hiding this comment

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

Pre-approved once #852 ships

Copy link
Contributor

@dleroux dleroux left a comment

Choose a reason for hiding this comment

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

Looks good but same comments as Tim over version and wording.

@BPScott BPScott temporarily deployed to polaris-react-pr-887 January 21, 2019 19:01 Inactive
@BPScott BPScott temporarily deployed to polaris-react-pr-887 January 21, 2019 19:10 Inactive
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.

4 participants