-
Notifications
You must be signed in to change notification settings - Fork 8
feat: Porting LexicalCharacterLimitPlugin #48
base: main
Are you sure you want to change the base?
feat: Porting LexicalCharacterLimitPlugin #48
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@@ -25,6 +25,7 @@ export type LexicalRichTextInitialState = | |||
<ng-content select="[lexicalContentEditable]"></ng-content> | |||
<ng-content select="[lexicalPlaceholder]"></ng-content> | |||
<ng-content select="[lexicalDecorators]"></ng-content> | |||
<ng-content select="lexical-character-limit"></ng-content> |
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.
Mmh this should not be needed since there is already a <ng-content> in the top
strlen?: (input: string) => number; | ||
}; | ||
|
||
export function useCharacterLimit( |
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 just my whim, what do you think about using different nomenclature? In Angular the use
prefix is not used as much as React.
We can think of it about a observer
callback since it follow the Unsubscribable interface like observable, but it's just an idea. Probably I will call it something like observeCharacterLimit 🤔. Also soon we could better manage this callback by making it more rxjs-like instead of defining it in this way
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.
Something like:
export function observeCharacterLimit(
editor: LexicalEditor,
maxCharacters: number,
optional: OptionalProps = Object.freeze({})
): MonoTypeOperatorFunction<[string, Parameters<UpdateListener>[0]]> {
const {
strlen = input => input.length,
// UTF-16
remainingCharacters = () => {
return;
},
} = optional;
let lastComputedTextLength = 0;
return pipe(
filter(([_, {dirtyLeaves}]) => {
const isComposing = editor.isComposing();
const hasDirtyLeaves = dirtyLeaves.size > 0;
return !(isComposing || !hasDirtyLeaves);
}),
tap(([text, _]) => {
const textLength = strlen(text);
const textLengthAboveThreshold =
textLength > maxCharacters ||
(lastComputedTextLength !== null &&
lastComputedTextLength > maxCharacters);
const diff = maxCharacters - textLength;
remainingCharacters(diff);
if (lastComputedTextLength === null || textLengthAboveThreshold) {
const offset = findOffset(text, maxCharacters, strlen);
editor.update(
() => {
$wrapOverflowedNodes(offset);
},
{
tag: 'history-merge',
}
);
}
lastComputedTextLength = textLength;
})
);
}
and...
this.listener = combineLatest([
this.controller.onTextContent$,
this.controller.onUpdate$,
])
.pipe(observeCharacterLimit(editor, characterLimit, characterLimitProps))
.subscribe();
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.
Mmh what about passing also the observables in the signature? 🤔 This may be more readable, but it's just an opinion
Anyway I think it's just that I'm not very used to seeing such custom operators since i use mostly creation functions, but it could be ok. I just think that MonoTypeOperatorFunction signature is a bit weird (think if we use this approach also for other more complex listener).
For example
export function observeCharacterLimit(
textContent$,
onUpdate$,
options: {
editor: LexicalEditor,
maxCharacters: number,
optional: OptionalProps = Object.freeze({})
}
)
and..
this.listener = observeCharacterLimit(
this.controller.onTextContent$,
this.controller.onUpdate$,
{
editor,
characterLimit,
characterLimitProps
}
).subscribe()
.characters-limit { | ||
color: #888; | ||
font-size: 12px; | ||
text-align: right; | ||
display: block; | ||
position: absolute; | ||
left: 12px; | ||
bottom: 5px; | ||
} | ||
|
||
.characters-limit.characters-limit-exceeded { | ||
color: red; | ||
} |
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.
Mm we should think about a better style considering hypothetically the personalization. Now, we can provide css variables to overwrite, otherwise we can give 100% of the style decisions to the consumer user.
Personally I would do something style agnostic for now providing the style decision to the consumer, less breakups in the future, but I am open to different opinions.
In this case this mean that these styles should be moved to playground
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.
Something like:
styles: [
`
.characters-limit {
color: var(--characters-limit-color, #888);
font-size: var(--characters-limit-font-size, 12px);;
text-align: right;
display: block;
position: absolute;
left: 12px;
bottom: 5px;
}
.characters-limit.characters-limit-exceeded {
color: var(--characters-limit-exceeded-color, red);
}
`,
],
and in playground css, we could overwrite those variables
:root {
--characters-limit-color: #818;
--characters-limit-exceeded-color: blue;
}
which properties could be overwritten? Remember it's possible to add a custom class to component here
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.
Reconsidering this, it may be better to attaching the classes characters-limit
and characters-limit-exceeded
and move these styles in the playground 🤔 This is what they are doing in their repo at the moment
Closes #43
New feature to porting LexicalCharacterLimitPlugin.