-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Integrate with ElementInternals
#1188
Changes from 1 commit
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,7 +1,5 @@ | ||
export default () => | ||
`<label id="label-1" for="editor"><span>Label 1</span></label> | ||
<label id="label-2"> | ||
Label 2 | ||
<trix-editor id="editor"></trix-editor> | ||
</label> | ||
<label id="label-3" for="editor">Label 3</label>` | ||
<label id="label-2">Label 2</label> | ||
<trix-editor id="editor"></trix-editor> | ||
<label id="label-3" for="editor">Label 3</label>` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,12 @@ | ||
export default () => | ||
`<form id="ancestor-form"> | ||
<trix-editor id="editor-with-ancestor-form"></trix-editor> | ||
<trix-editor id="editor-with-ancestor-form" name="editor-with-ancestor-form"></trix-editor> | ||
</form> | ||
|
||
<form id="input-form"> | ||
<input type="hidden" id="hidden-input"> | ||
</form> | ||
<trix-editor id="editor-with-input-form" input="hidden-input"></trix-editor> | ||
|
||
<trix-editor id="editor-with-no-form"></trix-editor>` | ||
<trix-editor id="editor-with-no-form"></trix-editor> | ||
<fieldset id="fieldset"><trix-editor id="editor-within-fieldset"></fieldset>` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,6 @@ | ||
import * as config from "trix/config" | ||
|
||
import { | ||
findClosestElementFromNode, | ||
handleEvent, | ||
handleEventOnce, | ||
installDefaultCSSForTagName, | ||
|
@@ -161,6 +160,14 @@ installDefaultCSSForTagName("trix-editor", `\ | |
}`) | ||
|
||
export default class TrixEditorElement extends HTMLElement { | ||
static formAssociated = true | ||
|
||
#internals | ||
|
||
constructor() { | ||
super() | ||
this.#internals = this.attachInternals() | ||
} | ||
|
||
// Properties | ||
|
||
|
@@ -174,19 +181,7 @@ export default class TrixEditorElement extends HTMLElement { | |
} | ||
|
||
get labels() { | ||
const labels = [] | ||
if (this.id && this.ownerDocument) { | ||
labels.push(...Array.from(this.ownerDocument.querySelectorAll(`label[for='${this.id}']`) || [])) | ||
} | ||
|
||
const label = findClosestElementFromNode(this, { matchingSelector: "label" }) | ||
if (label) { | ||
if ([ this, null ].includes(label.control)) { | ||
labels.push(label) | ||
} | ||
} | ||
|
||
return labels | ||
return this.#internals.labels | ||
} | ||
|
||
get toolbarElement() { | ||
|
@@ -238,6 +233,18 @@ export default class TrixEditorElement extends HTMLElement { | |
this.editor?.loadHTML(this.defaultValue) | ||
} | ||
|
||
get disabled() { | ||
return this.inputElement.disabled | ||
} | ||
|
||
set disabled(value) { | ||
this.toggleAttribute("disabled") | ||
} | ||
|
||
get type() { | ||
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. Is this just for testing purposes? Could you replace with just 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 isn't solely for testing. I've added the property because it's mentioned in the Defining a form-associated custom element section of the More capable form controls article. Having said that, it's implemented using Element.localName (and not hard-coded like I've done here). It also doesn't seem to be mentioned by the MDN documentation for ElementInternals, so it isn't clear how necessary it is to start. |
||
return "trix-editor" | ||
seanpdoyle marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
// Controller delegate methods | ||
|
||
notify(message, data) { | ||
|
@@ -269,54 +276,23 @@ export default class TrixEditorElement extends HTMLElement { | |
requestAnimationFrame(() => triggerEvent("trix-initialize", { onElement: this })) | ||
} | ||
this.editorController.registerSelectionManager() | ||
this.registerResetListener() | ||
this.registerClickListener() | ||
autofocus(this) | ||
} | ||
} | ||
|
||
disconnectedCallback() { | ||
this.editorController?.unregisterSelectionManager() | ||
this.unregisterResetListener() | ||
return this.unregisterClickListener() | ||
} | ||
|
||
// Form support | ||
|
||
registerResetListener() { | ||
this.resetListener = this.resetBubbled.bind(this) | ||
return window.addEventListener("reset", this.resetListener, false) | ||
} | ||
|
||
unregisterResetListener() { | ||
return window.removeEventListener("reset", this.resetListener, false) | ||
} | ||
|
||
registerClickListener() { | ||
this.clickListener = this.clickBubbled.bind(this) | ||
return window.addEventListener("click", this.clickListener, false) | ||
} | ||
|
||
unregisterClickListener() { | ||
return window.removeEventListener("click", this.clickListener, false) | ||
} | ||
|
||
resetBubbled(event) { | ||
if (event.defaultPrevented) return | ||
if (event.target !== this.form) return | ||
return this.reset() | ||
formDisabledCallback(disabled) { | ||
this.inputElement.disabled = disabled | ||
this.toggleAttribute("contenteditable", !disabled) | ||
} | ||
|
||
clickBubbled(event) { | ||
if (event.defaultPrevented) return | ||
if (this.contains(event.target)) return | ||
|
||
const label = findClosestElementFromNode(event.target, { matchingSelector: "label" }) | ||
if (!label) return | ||
|
||
if (!Array.from(this.labels).includes(label)) return | ||
|
||
return this.focus() | ||
formResetCallback() { | ||
this.reset() | ||
} | ||
|
||
reset() { | ||
|
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.
The "disabled" bit is separated from the "element internals" change, right? I like both, but are they related?
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.
The integration with
ElementInternals
provides access to the formDisabledCallback(disabled) lifecycle hook. Through that integration the element is given an opportunity to disable or re-enable its associated<input>
. The[disabled]
attribute is a common attribute among form controls, and provides the server an opportunity to render the element with an initially disabled state. However, the[disabled]
attribute is only one aspect of disabling an element. If it's rendered within a<fieldset disabled>
, theformDisabledCallback(disabled)
will fire (and the associated<input>
will be disabled anyway if its a child of the<fieldset>
whetherElementInternals
is integrated or not). There is also a :disabled CSS pseudo-selector that incorporates both[disabled]
on the element itself and:disabled
of any ancestor<fieldset>
element.The
.disabled
property itself is an enhancement that's separate from the bare necessities.I've included it here because applications that want to introspect whether or not the element is disabled are unlikely to get it right when incorporating
[disabled]
on the element,:disabled
on its<fieldset>
ancestors (or their HTMLFieldSetElement.disabled property).Since the
<input>
element is built-in, it already accounts for all these permutations. The currentformDisabledCallback(disabled)
implementation already manages theHTMLInputElement.disabled
property, so the<input>
felt like a good delegate for the time being. If the<trix-editor>
is ever changed to managed its own value in a way that makes the<input>
no longer necessary, this property could be changed to handle the rest of the logic.