-
Notifications
You must be signed in to change notification settings - Fork 600
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
TextEditor: 'outside' label should be aligned when before buttons used and input should be focused after label click #25952
Changes from all commits
6c35e89
82d825b
c2aa409
f9989fa
0ef8097
df1fbe6
ce9c740
aa87dd2
182a96e
b50dde1
ae59230
631e1ee
6717a70
be376a4
f34948a
4f24928
33b17a3
8feac40
fd2f908
5729cb2
c33beff
7dca140
ca75620
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,12 @@ | ||
import $ from '../../core/renderer'; | ||
import Guid from '../../core/guid'; | ||
import { name as click } from '../../events/click'; | ||
import eventsEngine from '../../events/core/events_engine'; | ||
import { addNamespace } from '../../events/utils/index'; | ||
import { start as hoverStart } from '../../events/hover'; | ||
import { active } from '../../events/core/emitter.feedback'; | ||
import { getWindow } from '../../core/utils/window'; | ||
import { getWidth } from '../../core/utils/size'; | ||
|
||
const TEXTEDITOR_LABEL_CLASS = 'dx-texteditor-label'; | ||
const TEXTEDITOR_WITH_LABEL_CLASS = 'dx-texteditor-with-label'; | ||
|
@@ -12,20 +19,9 @@ const LABEL_CLASS = 'dx-label'; | |
const LABEL_AFTER_CLASS = 'dx-label-after'; | ||
|
||
class TextEditorLabel { | ||
constructor({ | ||
$editor, | ||
text, mode, mark, | ||
containsButtonsBefore, | ||
containerWidth, | ||
beforeWidth | ||
}) { | ||
this._props = { | ||
$editor, | ||
text, mode, mark, | ||
containsButtonsBefore, | ||
containerWidth, | ||
beforeWidth | ||
}; | ||
constructor(props) { | ||
this.NAME = 'dxLabel'; | ||
this._props = props; | ||
|
||
this._id = `${TEXTEDITOR_LABEL_CLASS}-${new Guid()}`; | ||
|
||
|
@@ -69,6 +65,34 @@ class TextEditorLabel { | |
visible | ||
? this._$root.appendTo(this._props.$editor) | ||
: this._$root.detach(); | ||
|
||
this._attachEvents(); | ||
} | ||
|
||
_attachEvents() { | ||
const clickEventName = addNamespace(click, this.NAME); | ||
const hoverStartEventName = addNamespace(hoverStart, this.NAME); | ||
const activeEventName = addNamespace(active, this.NAME); | ||
|
||
eventsEngine.off(this._$labelSpan, clickEventName); | ||
eventsEngine.off(this._$labelSpan, hoverStartEventName); | ||
|
||
if(this._isVisible() && this._isOutsideMode()) { | ||
eventsEngine.on(this._$labelSpan, clickEventName, (e) => { | ||
const selectedText = getWindow().getSelection().toString(); | ||
|
||
if(selectedText === '') { | ||
this._props.onClickHandler(); | ||
e.preventDefault(); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This code seems strange for me. As far as i understand click in selected text will not trigger input's focus. I'm not sure it's ok. What if selection is somewhere not in label? Maybe we can user pointerup here or detect selection other way? |
||
}); | ||
eventsEngine.on(this._$labelSpan, hoverStartEventName, (e) => { | ||
this._props.onHoverHandler(e); | ||
}); | ||
eventsEngine.on(this._$labelSpan, activeEventName, (e) => { | ||
this._props.onActiveHandler(e); | ||
}); | ||
} | ||
} | ||
|
||
_updateEditorLabelClass(visible) { | ||
|
@@ -84,12 +108,16 @@ class TextEditorLabel { | |
|
||
this._props.$editor.addClass(labelClass); | ||
|
||
if(this._props.mode === 'outside') { | ||
if(this._isOutsideMode()) { | ||
this._props.$editor.addClass(TEXTEDITOR_LABEL_OUTSIDE_CLASS); | ||
} | ||
} | ||
} | ||
|
||
_isOutsideMode() { | ||
return this._props.mode === 'outside'; | ||
} | ||
|
||
_updateEditorBeforeButtonsClass(visible = this._isVisible()) { | ||
this._props.$editor | ||
.removeClass(TEXTEDITOR_WITH_BEFORE_BUTTONS_CLASS); | ||
|
@@ -111,13 +139,24 @@ class TextEditorLabel { | |
|
||
_updateBeforeWidth() { | ||
this._$before.css({ width: this._props.beforeWidth }); | ||
|
||
this._updateLabelTransform(); | ||
} | ||
|
||
_updateLabelTransform(offset = 0) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's sad that we need js calculations again for labels width but I don't see a good approach to implement if fast another way. So let's leave it as it is and create a separate card for solution's rework. |
||
this._$labelSpan.css('transform', ''); | ||
|
||
if(this._isVisible() && this._isOutsideMode()) { | ||
const sign = this._props.rtlEnabled ? 1 : -1; | ||
|
||
this._$labelSpan.css('transform', 'translateX(' + sign * (getWidth(this._$before) + offset) + 'px)'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's move offset's calculation result to a separate variable to improve readability. |
||
} | ||
} | ||
|
||
_updateMaxWidth() { | ||
this._$label.css({ maxWidth: this._props.containerWidth }); | ||
} | ||
|
||
|
||
$element() { | ||
return this._$root; | ||
} | ||
|
@@ -133,6 +172,7 @@ class TextEditorLabel { | |
updateMode(mode) { | ||
this._props.mode = mode; | ||
this._toggleMarkupVisibility(); | ||
this._updateLabelTransform(); | ||
} | ||
|
||
updateText(text) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +0,0 @@ | ||
@use "../icons" as *; | ||
|
||
@mixin colorbox-label-outside-mixin() { | ||
&.dx-texteditor-label-outside { | ||
&.dx-editor-filled, | ||
&.dx-editor-underlined { | ||
.dx-label-before { | ||
min-width: 0; | ||
} | ||
} | ||
} | ||
} | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,8 +40,6 @@ | |
} | ||
} | ||
} | ||
|
||
@include colorbox-label-outside-mixin(); | ||
} | ||
|
||
.dx-colorbox-color-result-preview { | ||
|
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.
why don't we unsubscribe active handler?