Skip to content
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

[lexical-playground] Fix bug: Insert an image inside another image's caption (#6109) #6230

Merged
merged 1 commit into from
Jun 12, 2024

Conversation

MaxPlav
Copy link
Contributor

@MaxPlav MaxPlav commented May 31, 2024

Context:
Now users are able to insert an image/GIF/table etc, into another image's caption.

Solution:
This PR limits what the users can insert into the image caption and its nested editor. Excluding some controls from the toolbar if the selection is inside the nested editor that belongs to the image caption.

Closes #6109

Test plan

  1. Open https://playground.lexical.dev/
  2. Insert a sample image with a caption.
  3. Focus on the caption.
  4. Insert a sample image/table/GIF etc.

Before

before-image-caption.mov

After

image-caption.mov

Copy link

vercel bot commented May 31, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
lexical ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 10, 2024 5:35pm
lexical-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 10, 2024 5:35pm

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 31, 2024
Copy link

github-actions bot commented May 31, 2024

size-limit report 📦

Path Size
lexical - cjs 28.31 KB (0%)
lexical - esm 28.13 KB (0%)
@lexical/rich-text - cjs 36.77 KB (0%)
@lexical/rich-text - esm 28.09 KB (0%)
@lexical/plain-text - cjs 35.36 KB (0%)
@lexical/plain-text - esm 25.33 KB (0%)
@lexical/react - cjs 38.51 KB (0%)
@lexical/react - esm 29.14 KB (0%)

@MaxPlav
Copy link
Contributor Author

MaxPlav commented Jun 4, 2024

update: the image caption nested editor uses PlainTextPlugin so it can paste the content as plain text - similar behavior as for a Sticky Note node. Fixed the issue so that an image/GIF/table etc. can be pasted to an image caption.

@MaxPlav
Copy link
Contributor Author

MaxPlav commented Jun 5, 2024

update: resolved an issue with non-functional text formatting and alignment in the image caption nested editor.

image_caption_formatting_fix.mov

@MaxPlav MaxPlav marked this pull request as ready for review June 5, 2024 19:44
setActiveEditor(newEditor);
$updateToolbar(newEditor);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you don't need to pass newEditor to $updateToolbar() as you can grab it from activeEditor var from the component scope

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure why but the function has a stale activeEditor value sometimes, so passing as an argument

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you don't need to pass newEditor to $updateToolbar() as you can grab it from activeEditor var from the component scope

thanks, updated that

(newEditor: LexicalEditor) => {
const selection = $getSelection();
if ($isRangeSelection(selection)) {
if (newEditor !== editor && $isEditorIsNestedEditor(newEditor)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (newEditor !== editor && $isEditorIsNestedEditor(newEditor)) {
if (activeEditor !== editor && $isEditorIsNestedEditor(activeEditor)) {

/**
* Checks if the editor is a nested editor created by LexicalNestedComposer
*/
export function $isEditorIsNestedEditor(editor: LexicalEditor): boolean {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of having this function you can simply check if editor._parentEditor is non null :)

Although maybe we still can have helper...

@@ -74,6 +74,7 @@ import {Dispatch, useCallback, useEffect, useState} from 'react';
import * as React from 'react';
import {IS_APPLE} from 'shared/environment';

import {$isEditorIsNestedEditor} from '../../../../lexical/src/LexicalUtils';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can't really import non-exported file from lexical package here

But you can move it to @lexical/utils

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Copy link
Contributor

@StyleT StyleT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. extended-tests Run extended e2e tests on a PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: Insert an image inside another image's caption
3 participants