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

support notification pop up from topLeft or bottomRight or bottomLeft #4700

Merged
merged 7 commits into from
Jan 27, 2017
Merged

support notification pop up from topLeft or bottomRight or bottomLeft #4700

merged 7 commits into from
Jan 27, 2017

Conversation

ystarlongzi
Copy link
Contributor

closed #4531

@afc163 @benjycui 调整涉及到的内容如下:

  1. 参照 tooltip,引入了 placement 接口,可以在四个方位弹出消息
  2. 根据 @afc163pull 4675 处的建议,引入了 bottom 接口,控制消息距离浏览器底部的距离
  3. 更新了一些 snapshots

@mention-bot
Copy link

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

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.9%) to 76.228% when pulling d11c942a3402fa4326e64603a7dd7723b2a24d70 on ystarlongzi:notification-placement into 70188ae on ant-design:feature-2.7.

@ystarlongzi
Copy link
Contributor Author

@afc163 @benjycui

  1. travis 和 本地运行 npm run test-all 得到的结果不一致。本地也有报错,但信息如下:
Summary of all failing tests
 FAIL  components/auto-complete/__tests__/ac.test.js
  ● AutoComplete with Custom Input Element Render › AutoComplete with custom Input render perfectly

    expect(received).toBe(expected)
    
    Expected value to be (using ===):
      1
    Received:
      0
      
      at Object.<anonymous> (components/auto-complete/__tests__/ac.test.js:13:45)
      at handle (node_modules/worker-farm/lib/child/index.js:41:8)
      at process.<anonymous> (node_modules/worker-farm/lib/child/index.js:47:3)
      at emitTwo (events.js:106:13)
      at process.emit (events.js:191:7)
      at process.nextTick (internal/child_process.js:719:12)
      at _combinedTickCallback (internal/process/next_tick.js:67:7)
      at process._tickCallback (internal/process/next_tick.js:98:9)

 FAIL  components/form/__tests__/demo.test.js
  ● renders ./components/form/demo/horizontal-login.md correctly

    TypeError: isFieldTouched is not a function
      
      at render (components/form/demo/horizontal-login.md:60:25)
      at node_modules/react-dom/lib/ReactCompositeComponent.js:796:21
      at measureLifeCyclePerf (node_modules/react-dom/lib/ReactCompositeComponent.js:75:12)
      at ReactCompositeComponent._renderValidatedComponentWithoutOwnerOrContext (node_modules/react-dom/lib/ReactCompositeComponent.js:795:25)
      at ReactCompositeComponent._renderValidatedComponent (node_modules/react-dom/lib/ReactCompositeComponent.js:822:32)
      at ReactCompositeComponent.performInitialMount (node_modules/react-dom/lib/ReactCompositeComponent.js:362:30)
      at ReactCompositeComponent.mountComponent (node_modules/react-dom/lib/ReactCompositeComponent.js:258:21)
      at Object.ReactReconciler.mountComponent (node_modules/react-dom/lib/ReactReconciler.js:46:35)
      at ReactCompositeComponent.performInitialMount (node_modules/react-dom/lib/ReactCompositeComponent.js:371:34)
      at ReactCompositeComponent.mountComponent (node_modules/react-dom/lib/ReactCompositeComponent.js:258:21)
      at Object.ReactReconciler.mountComponent (node_modules/react-dom/lib/ReactReconciler.js:46:35)
      at ReactCompositeComponent.performInitialMount (node_modules/react-dom/lib/ReactCompositeComponent.js:371:34)
      at ReactCompositeComponent.mountComponent (node_modules/react-dom/lib/ReactCompositeComponent.js:258:21)
      at Object.ReactReconciler.mountComponent (node_modules/react-dom/lib/ReactReconciler.js:46:35)
      at node_modules/react-dom/lib/ReactServerRendering.js:45:36
      at ReactServerRenderingTransaction.TransactionImpl.perform (node_modules/react-dom/lib/Transaction.js:140:20)
      at renderToStringImpl (node_modules/react-dom/lib/ReactServerRendering.js:43:24)
      at renderToStaticMarkup (node_modules/react-dom/lib/ReactServerRendering.js:83:10)
      at render (node_modules/enzyme/build/render.js:27:52)
      at Object.<anonymous> (tests/shared/demoTest.js:17:66)


Test Suites: 2 failed, 1 skipped, 73 passed, 75 of 76 total
Tests:       2 failed, 10 skipped, 373 passed, 385 total
Snapshots:   315 passed, 315 total
Time:        23.77s
  1. 求教关于 Coverage decreased 的失败,我要怎么去解决?

@@ -22,8 +22,9 @@ exports[`test renders ./components/auto-complete/demo/antd.md correctly 1`] = `
class="ant-select-search ant-select-search--inline">
<div
class="ant-select-search__field__wrap">
<textarea
class="ant-input ant-select-search__field" />
Copy link
Member

Choose a reason for hiding this comment

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

这里不应该修改。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

snapshots 是在运行 npm test -- -u 后生成的

@afc163
Copy link
Member

afc163 commented Jan 24, 2017

非 notification 目录下的 snapshot 不应该有改动。你本地 node_modules 恐怕不是最新的。

@ystarlongzi
Copy link
Contributor Author

确实不是最新的,我再运行下命令。👍

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.9%) to 76.228% when pulling 53d3d4615b9929a735b3a1208f956af36891769a on ystarlongzi:notification-placement into 70188ae on ant-design:feature-2.7.

@afc163
Copy link
Member

afc163 commented Jan 24, 2017

再 rebase 一下,steps 也不应该有改动。:sweat_smile:

onChange={val => {
notification.config({
placement: val,
});
Copy link
Member

Choose a reason for hiding this comment

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

没对齐。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的,我调整下。

const openNotification = () => {
notification.open({
message: 'Notification Title',
description: 'This is the content of the notification. This is the content of the notification. This is the content of the notification.',
Copy link
Member

@afc163 afc163 Jan 24, 2017

Choose a reason for hiding this comment

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

open 这里也支持一下 placement 如何?

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 Author

Choose a reason for hiding this comment

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

@afc163
在实现上支持 placement,然后 demo 里的 open 方法内就不用体现 placement 了吧,因为 demo 里是通过 config 方法修改 placement

Copy link
Member

Choose a reason for hiding this comment

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

那文档里补充一下。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

嗯嗯,好的。

{val}
</Option>
);
})}
Copy link
Member

Choose a reason for hiding this comment

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

{options.map(val => <Option key={val}>{val}</Option>)}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的,我调整下。

@afc163
Copy link
Member

afc163 commented Jan 24, 2017

rebase 一下,最好不要出现 Merge branch 的提交。

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.04%) to 76.05% when pulling b1855d5850ba828696c36d693a2aa206e7de1085 on ystarlongzi:notification-placement into f4e887b on ant-design:feature-2.7.

@afc163
Copy link
Member

afc163 commented Jan 24, 2017

还有,补一下用例吧,真是辛苦了。。

@ystarlongzi
Copy link
Contributor Author

嗯嗯,好的,我也研究下 rebase 的用法。

@afc163
Copy link
Member

afc163 commented Jan 25, 2017

@ystarlongzi 节前有时间完成么?春节前我们要发 2.7 ~

@afc163 afc163 changed the title support notification pop up from topLeft or bottomRight or bottomLeft [WIP] support notification pop up from topLeft or bottomRight or bottomLeft Jan 25, 2017
@ystarlongzi
Copy link
Contributor Author

@afc163
嗯嗯,在了,刚到家。
现在我有个难点还是 rebase 的使用,看了些教程和文档,但是实际操作起来还是摸不着头脑。
我先写下用例吧。

@afc163
Copy link
Member

afc163 commented Jan 26, 2017

$ git checkout feature-2.7
$ git pull --rebase git@github.com:ant-design/ant-design.git feature-2.7   // 把 feature-2.7 更新到最新
$ git checkout notification-placement
$ git rebase feature-2.7

然后如果冲突了。用 git status 查看冲突文件,本地编辑手动解决所有冲突。

$ git add {解决完冲突的文件}
$ git rebase --continue // 继续 rebase,如果冲突返回上面继续解决
$ git push origin notification-placement -f  // 强制 push 到远程分支

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.9%) to 75.755% when pulling a7b2e3f on ystarlongzi:notification-placement into fd233cf on ant-design:feature-2.7.

@benjycui
Copy link
Contributor

空了可以看下 pro git 之类的书籍。

@RaoHai
Copy link
Contributor

RaoHai commented Jan 26, 2017

2. update doc
3. support `placement` arguments in `open` method
@ystarlongzi
Copy link
Contributor Author

@afc163 @benjycui 谢谢!假期会再了解下关于 git 的内容。
@RaoHai 谢谢提供的文章!

@coveralls
Copy link

Coverage Status

Coverage increased (+1.9%) to 78.516% when pulling 546a789 on ystarlongzi:notification-placement into fd233cf on ant-design:feature-2.7.

@afc163 afc163 changed the title [WIP] support notification pop up from topLeft or bottomRight or bottomLeft support notification pop up from topLeft or bottomRight or bottomLeft Jan 26, 2017
| top | 消息距离顶部的位置 | Number | 24px |
| placement | 弹出位置,可选 `topLeft` `topRight` `bottomLeft` `bottomRight` | string | topRight |
| top | 消息从顶部弹出时,距离顶部的位置 | Number | 24px |
| bottom | 消息从底部弹出时,距离底部的位置 | Number | 24px |
Copy link
Member

Choose a reason for hiding this comment

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

number 小写,默认值是 24px 还是 24 ?如果是 24px 的话那应该是 string

Copy link
Contributor Author

@ystarlongzi ystarlongzi Jan 27, 2017

Choose a reason for hiding this comment

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

24,我调整下。

@coveralls
Copy link

Coverage Status

Coverage increased (+1.9%) to 78.516% when pulling c6b0780 on ystarlongzi:notification-placement into fd233cf on ant-design:feature-2.7.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.9%) to 78.516% when pulling c6b0780 on ystarlongzi:notification-placement into fd233cf on ant-design:feature-2.7.

@afc163 afc163 merged commit aadf623 into ant-design:feature-2.7 Jan 27, 2017
@afc163 afc163 added the ❓FAQ issues would be discussed a lot label Oct 26, 2018
@orzyyyy orzyyyy mentioned this pull request Oct 20, 2019
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
❓FAQ issues would be discussed a lot
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants