-
Notifications
You must be signed in to change notification settings - Fork 660
Chat.Editing: contextmenu integration #29672
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
Chat.Editing: contextmenu integration #29672
Conversation
contextMenu: support behavior when scrolling + positioning
531d607 to
e29ccfc
Compare
packages/devextreme/testing/tests/DevExpress.ui.widgets/chatParts/messageList.tests.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like there is an issue with verical centering in generic
| onMessageDeleting, | ||
| } = this.option(); | ||
|
|
||
| const editText = messageLocalization.format('dxChat-editingEditMessage'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's bette to place it near the place of usage inside of if. Now we get this data even when it's no needed
| hideOnParentScroll: false, | ||
| overlayContainer: this._scrollView.content(), | ||
| visualContainer: this._scrollView.container(), | ||
| boundaryOffset: { h: 16 }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's create a constant SCROLLBAR_WIDTH to explain this magic number
| onCloseRootSubmenu: null, | ||
| onExpandLastSubmenu: null, | ||
| hideOnParentScroll: true, | ||
| visualContainer: window, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a _ prefix to point out that it's private properties
And also declare overlayContainer and boundaryOffset here as well just for consistency
| }); | ||
|
|
||
| QUnit.module('onMessageDeleting', moduleConfig, () => { | ||
| QUnit.test('should be called when the Edit button is clicked', function(assert) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Delete button
| cssClass: CHAT_MESSAGELIST_CONTEXT_MENU_CONTENT_CLASS, | ||
| visible: false, | ||
| overlayContainer: this.getScrollView().content(), | ||
| visualContainer: this.getScrollView().container(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You forgot boundaryOffset
| }); | ||
|
|
||
| QUnit.module('OnMessageEditingStart', moduleConfig, () => { | ||
| QUnit.test('should be called when the Edit button is clicked', function(assert) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it a copypaste from messageList tests?
Co-authored-by: ksercs <ksercs0@gmail.com>
No description provided.