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

feat(message provider): add placement prop #851

Merged

Conversation

doom-9-zz
Copy link
Contributor

No description provided.

doom-9 and others added 30 commits June 17, 2021 17:42
@@ -56,6 +56,7 @@ multiple-line
| closable | `boolean` | All messages whether to show close icon. |
| duration | `number` | `3000` | All messages's default duration. |
| max | `number` | `undefined` | Limit the number of message to display. |
| placement | `top \| top-left \| top-right \| bottom \| bottom-left \| bottom-right ` | `top` | All messages placement |
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
| placement | `top \| top-left \| top-right \| bottom \| bottom-left \| bottom-right ` | `top` | All messages placement |
| placement | `top \| top-left \| top-right \| bottom \| bottom-left \| bottom-right ` | `top` | All message's placement. |

Comment on lines 126 to 131
style={{
...(cssVars as CSSProperties),
[this.placement.startsWith('top') ? 'top' : 'bottom']: '12px',
left: this.placement.endsWith('left') ? '12px' : '',
right: this.placement.endsWith('right') ? '12px' : ''
}}
Copy link
Contributor

Choose a reason for hiding this comment

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

应该可以放到container上

Copy link
Contributor Author

Choose a reason for hiding this comment

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

我刚开始放到container上,有些样式问题

@doom-9-zz doom-9-zz changed the title feat(message provider): add placement prop(wip) feat(message provider): add placement prop Aug 12, 2021
@@ -26,6 +26,7 @@ import fadeInHeightExpand from '../../../_styles/transitions/fade-in-height-expa
// --border-radius
export default c([
cB('message-wrapper', `
position: fixed;
Copy link
Collaborator

Choose a reason for hiding this comment

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

不能这么弄,若干这样没办法叠加了。

我们需要改的是 container 的样式,不是 message 自身。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

晓得了,我改下

Comment on lines 7 to 8
- `n-message-provider` add `container-style` prop
- `n-message-provider` add `placement` prop.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- `n-message-provider` add `container-style` prop
- `n-message-provider` add `placement` prop.
- `n-message-provider` add `container-style` prop.
- `n-message-provider` add `placement` prop.

@@ -21,10 +21,10 @@ import { defineComponent } from 'vue'

// content
export default defineComponent({
setup () {
setup() {
Copy link
Contributor

Choose a reason for hiding this comment

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

please use eslint to format it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这个格式化是commit触发的钩子

src/message/src/styles/index.cssr.ts Show resolved Hide resolved
src/message/src/styles/index.cssr.ts Show resolved Hide resolved
@07akioni
Copy link
Collaborator

现在 Message 本体的动画有问题,从上往下的时候,折叠过程中 message 按照上边缘对齐的,所以 message 的位置看起来不会有很大的 offset。但是如果从下到上,message 的动画还是按照上边缘对齐,就会往下走了。

@doom-9-zz
Copy link
Contributor Author

现在 Message 本体的动画有问题,从上往下的时候,折叠过程中 message 按照上边缘对齐的,所以 message 的位置看起来不会有很大的 offset。但是如果从下到上,message 的动画还是按照上边缘对齐,就会往下走了。

确实,我处理下

@@ -106,10 +106,11 @@ export default defineComponent({
'--line-height': lineHeight,
'--border-radius': borderRadius
}
})
}),
placement: messageProviderProps.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
Collaborator

Choose a reason for hiding this comment

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

不用,先不支持单个 message 的位置。有需求我们在加,那样会比较复杂,需要每个 placement 储存一个数组。

const messages = {
  top: [],
  bottom: [],
  ...
}

我们先暂时不做

@@ -150,7 +156,7 @@ export default defineComponent({
}
})

function createIconVNode (
function createIconVNode(
Copy link
Collaborator

Choose a reason for hiding this comment

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

lint 有点问题,我先合了

Copy link
Collaborator

@07akioni 07akioni Aug 15, 2021

Choose a reason for hiding this comment

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

以后注意一下就行

@07akioni 07akioni merged commit 3d7f96d into tusen-ai:main Aug 15, 2021
rhengles pushed a commit to arijs/naive-ui that referenced this pull request Oct 20, 2021
* feat:n-input Support hidden password

* feat(form): support require-mark-placement(tusen-ai#171)

* Revert "feat(form): support require-mark-placement(tusen-ai#171)"

This reverts commit 0627777.

* Revert "feat:n-input Support hidden password"

This reverts commit ea64917.

* feat(message provider): add placement prop(wip)

* feat(message provider): add placement prop

* feat: add demo

* feat: fix demo code

* feat: fix code

* feat: fix code

* Update CHANGELOG.en-US.md

* feat: fix style code
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