Feat: [Regions] contenteditable content

[kakashy]

Short description

Resolves #3404

Implementation details

Added a contenteditable property to the content generated by the Regions plugin. This property is false by default.

How to test it

Create a new region and tap/click on the text content of the region.

Screenshots

Checklist

• This PR is covered by e2e tests
• It introduces no breaking API changes

[katspaugh]

Nani?

[kakashy]

Oh no! That's embarrasing :D

[katspaugh]

Suggested change

	if (this.contentEditable && this.content) {
	this.content.addEventListener('click', (e) => this.onContentClick(e))
	}
	if (this.contentEditable && this.content) {
	this.content.addEventListener('blur', (e) => this.onContentBlur(e))
	}
	if (this.contentEditable && this.content) {
	this.content.addEventListener('click', (e) => this.onContentClick(e))
	this.content.addEventListener('blur', (e) => this.onContentBlur(e))
	}

[katspaugh]

Looks good to me overall, left a couple of comments.

As an alternative to the onblur event you could also use onchange, I think. Feels a little more appropriate.

[katspaugh]

Why is this necessary btw? Isn't focusing the default action on click on editable elements?

[kakashy]

True, but with the region container having an event listener for clicks, this (focus on text content) does not happen. The event is immediately propagated to its parent. This function is in hopes that we can step in and handle the content property before propagating the event further. In the latest commit, I have made changes to emit the region-clicked event:

private onContentClick(event: MouseEvent) {
    event.stopPropagation()
    const contentContainer = event.target as HTMLDivElement
    contentContainer.focus()
    this.emit('click', event)
  }

[kakashy]

Thanks for the reviews. I have updated my PR with changes from the main repo along with the suggestions in your notes.

One thing that I am still thinking of is the fact that clicking on a text content and clicking away would trigger a region-updated event even if the text content remains the same. Perhaps a placeholder variable for the text content could help but this might end up cluttering the plugin file.

[katspaugh]

Thank you!