Trigger plugin #3268

civing · 2024-10-28T16:00:23Z

Adds triggers to be used when detecting a certain string and starting a trigger session. A trigger session begins when the magic string sequence is inserted, for instance an @ and will keep on sending user input back to the consumer until certain stop conditions occur.

Fixes: Lundalogik/crm-feature#4430
Fixes: https://github.com/Lundalogik/crm-feature/issues/4060
Co-authored-by: John Traas [email protected]

Review:

Commits are atomic
Commits have the correct type for the changes made
Commits with breaking changes are marked as such

Browsers tested:

(Check any that applies, it's ok to leave boxes unchecked if testing something didn't seem relevant.)

Windows:

Chrome
Edge
Firefox

Linux:

Chrome
Firefox

macOS:

Chrome
Firefox
Safari

Mobile:

Chrome on Android
iOS

github-actions · 2024-10-28T16:02:57Z

Documentation has been published to https://lundalogik.github.io/lime-elements/versions/PR-3268/

src/components/text-editor/prosemirror-adapter/plugins/trigger-plugin.ts

src/components/text-editor/text-editor.tsx

src/components/text-editor/prosemirror-adapter/plugins/trigger-plugin.ts

jgroth · 2024-10-31T12:47:15Z

src/components/text-editor/examples/text-editor-custom-triggers.tsx

+        );
+
+        return [
+            <limel-portal


The portal is a private component, so I don't think we should use it in an example since that will be very confusing for the consumers

jgroth · 2024-10-31T12:48:51Z

src/components/text-editor/examples/text-editor-custom-triggers.tsx

+    shadow: true,
+    styleUrl: 'text-editor-custom-triggers.scss',
+})
+export class TextEditorCustomTriggersExample {


There's a lot going on in this example, is it possible to simplify it a bit? There's usually a lot of copy/pasting going on from these examples, so we want them to be as easy to understand as possible

@LucyChyzhova was adding a lot of good things but yeah, maybe we can simplify

sure! will check what we can do

jgroth · 2024-10-31T13:02:22Z

src/components/text-editor/examples/text-editor-custom-triggers.tsx

+export class TextEditorCustomTriggersExample {
+    constructor() {
+        this.portalId = createRandomString();
+        this.globalClickListener = this.globalClickListener.bind(this);


We don't need explicit binding like this if we use an arrow function instead

src/components/text-editor/prosemirror-adapter/plugins/trigger-plugin.ts

jgroth · 2024-10-31T13:29:06Z

src/components/text-editor/prosemirror-adapter/plugins/trigger-plugin.ts

+const sendTriggerEvent = (
+    type: 'triggerStart' | 'triggerStop' | 'triggerChange',
+    view: EditorView,
+    trigger: TriggerCharacter,
+    value: string,
+) => {
+    const event = new CustomEvent<TriggerEventDetail>(type, {
+        detail: getTriggerEventDetail(view, trigger, value),
+        bubbles: true,
+        composed: true,
+    });
+    view.dom.dispatchEvent(event);
+};


This might be fine, but I think it would be better if we emit the events from the component instead. We can have a callback on the component that we invoke from here that emits the events, e.g.

class TextEditor { @Event() public triggerStart: EventEmitter<TriggerEventDetail>; private emitTriggerStart(data) { this.triggerStart.emit(...); } }

src/components/text-editor/prosemirror-adapter/plugins/trigger-plugin.ts

hannahu · 2024-11-01T08:47:28Z

src/components/text-editor/prosemirror-adapter/plugins/trigger/factory.ts

+    transactions.forEach((transaction) => {
+        if (transaction.docChanged) {
+            transaction.steps.forEach((step) => {
+                if (


Still some nested code, and also unnecessary inline code comments, but I'm ok merging it and get it in if it works as expected 😄

Change the name to `CustomElementDefinition` to better reflect the meaning of this type and how it's supposed to be used.

Add types for trigger functionality

Adds triggers to be used when detecting a certain string and starting a trigger session. A trigger session begins with the magic string sequence is inserted, for instance an @ and will keep on sending user input back to the consumer until certain stop conditions occur. fixes: Lundalogik/crm-feature#4430 Co-authored-by: John Traas <[email protected]>

Change the name of the property `plugins` to `customElements`. The idea is the be more explicit and less generic.

lime-opensource · 2024-11-01T13:11:57Z

🎉 This PR is included in version 37.65.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

adrianschmidt · 2024-11-19T13:14:51Z

src/components/text-editor/prosemirror-adapter/plugins/trigger/factory.ts

+            triggerText = '';
+            triggerPosition = state.selection.$from.pos - triggerText.length;
+            sendTriggerEvent('triggerStart', view, activeTrigger, triggerText);


I came across this when working on #3307. What's triggerText for, and is it supposed to ever be anything other than an empty string? If not, we can make the following change:

triggerText = ''; - triggerPosition = state.selection.$from.pos - triggerText.length; + triggerPosition = state.selection.$from.pos; sendTriggerEvent('triggerStart', view, activeTrigger, triggerText);

On the other hand, since it's a variable everywhere, it kinda looks like the intention is that it's not always going to remain an empty string? 🙂

We used it on line 193 in this file. :)
So no, we can't change the code according to your proposal. That would mean that the triggerPosition would be moved as you type and mess with the implementation.

if (updatedText !== triggerText) { triggerText = updatedText; sendTriggerEvent( 'triggerChange', pluginView, activeTrigger, triggerText.slice(1), ); }

jgroth reviewed Oct 30, 2024

View reviewed changes

src/components/text-editor/prosemirror-adapter/plugins/trigger-plugin.ts Outdated Show resolved Hide resolved

civing force-pushed the trigger-plugin branch 2 times, most recently from 194963a to e4f4e39 Compare October 31, 2024 07:19

civing marked this pull request as ready for review October 31, 2024 07:20

civing force-pushed the trigger-plugin branch from e4f4e39 to 29d1df5 Compare October 31, 2024 10:31

hannahu reviewed Oct 31, 2024

View reviewed changes

src/components/text-editor/prosemirror-adapter/plugins/trigger-plugin.ts Outdated Show resolved Hide resolved

hannahu reviewed Oct 31, 2024

View reviewed changes

src/components/text-editor/prosemirror-adapter/plugins/trigger-plugin.ts Outdated Show resolved Hide resolved

jgroth reviewed Oct 31, 2024

View reviewed changes

hannahu reviewed Nov 1, 2024

View reviewed changes

fix(custom-element): change name to CustomElementDefinition

545aa9c

Change the name to `CustomElementDefinition` to better reflect the meaning of this type and how it's supposed to be used.

civing force-pushed the trigger-plugin branch 2 times, most recently from 16cc82a to 5ac9bb0 Compare November 1, 2024 12:38

civing requested a review from a team as a code owner November 1, 2024 12:38

civing requested a review from jgroth November 1, 2024 12:42

civing and others added 5 commits November 1, 2024 13:47

feat(text-editor): add types

29d464a

Add types for trigger functionality

docs(text-editor): Add example for custom trigger

d33c9d2

refactor(text-editor): change name of plugins to customElements

d2d71d9

Change the name of the property `plugins` to `customElements`. The idea is the be more explicit and less generic.

fix: update api definitions

d8ebb50

civing force-pushed the trigger-plugin branch from 5ac9bb0 to d8ebb50 Compare November 1, 2024 12:59

jgroth approved these changes Nov 1, 2024

View reviewed changes

Merge branch 'main' into trigger-plugin

c0cd9e7

civing enabled auto-merge (rebase) November 1, 2024 13:05

civing merged commit 70c26b6 into main Nov 1, 2024

civing deleted the trigger-plugin branch November 1, 2024 13:08

lime-opensource added the released label Nov 1, 2024

adrianschmidt reviewed Nov 19, 2024

View reviewed changes

Trigger plugin #3268

Trigger plugin #3268

Uh oh!

Conversation

civing commented Oct 28, 2024 • edited by john-traas Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review:

Browsers tested:

Uh oh!

github-actions bot commented Oct 28, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lime-opensource commented Nov 1, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

civing Nov 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

civing commented Oct 28, 2024 •

edited by john-traas

Loading

civing Nov 19, 2024 •

edited

Loading