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

imporve the user experience of the official site #4457

Merged
merged 3 commits into from Jan 8, 2017
Merged

imporve the user experience of the official site #4457

merged 3 commits into from Jan 8, 2017

Conversation

chongshengsun
Copy link
Contributor

@chongshengsun chongshengsun commented Jan 3, 2017

Close: #4302

@mention-bot
Copy link

@pinggod, thanks for your PR! By analyzing the history of the files in this pull request, we identified @benjycui, @afc163 and @waywardmonkeys to be potential reviewers.

@chongshengsun chongshengsun changed the title normalize code based on lint message imporve the user experience of the official site Jan 3, 2017
const menu = [
<Button className="lang" type="ghost" size="small" onClick={this.handleLangChange} key="lang">
<FormattedMessage id="app.header.lang" />
</Button>,
<Menu mode={menuMode} selectedKeys={[activeMenuItem]} id="nav" key="nav">
<Menu.Item key="home">
<Link to={utils.getLocalizedPathname('/', isZhCN)}>
<Link onClick={this.handleHideMenu} to={utils.getLocalizedPathname('/', isZhCN)}>
Copy link
Member

Choose a reason for hiding this comment

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

这样绑定貌似略多余,试试监听 url 变化如何?

remix-run/react-router#964 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

嗯嗯 我明天试一下

Copy link
Contributor

Choose a reason for hiding this comment

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

bisheng 把 browserHistory 封装起来了,用这种方法监听吧: https://github.com/ant-design/ant-design/blob/master/site/theme/template/Layout/index.jsx#L41

@coveralls
Copy link

Coverage Status

Coverage remained the same at 76.842% when pulling 9395dcc on pinggod:issue-4302 into d4993ce on ant-design:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 76.842% when pulling 11803cf on pinggod:issue-4302 into d4993ce on ant-design:master.

@@ -22,7 +24,9 @@ export default class Header extends React.Component {
require('enquire.js')
.register('only screen and (min-width: 320px) and (max-width: 1024px)', {
match: () => {
this.setState({ menuMode: 'inline' });
this.setState({ menuMode: 'inline' }, () => {
this.context.router.listen(this.handleHideMenu);
Copy link
Contributor

Choose a reason for hiding this comment

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

如果用户不断的 resize 浏览器,match 方法会反复执行的,这样你就监听同一个事件多次了。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

是否有方式可以取消监听?

Copy link
Contributor

Choose a reason for hiding this comment

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

componentDidMount 里面监听不可以么?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

在 componentDidMount 中监听可以实现,只是纯粹的想了解一下是否有取消监听的方式。

@coveralls
Copy link

Coverage Status

Coverage remained the same at 76.842% when pulling 37e8bd9 on pinggod:issue-4302 into d4993ce on ant-design:master.

@chongshengsun
Copy link
Contributor Author

这个还有什么需要修改的吗?

@afc163
Copy link
Member

afc163 commented Jan 7, 2017

@benjycui

@afc163 afc163 merged commit 456860f into ant-design:master Jan 8, 2017
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

5 participants