-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
fix: end-of-line cursor issue in Safari #6282
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
size-limit report 📦
|
c37f9f3
to
fecb88c
Compare
Hi! Can you pls submit screen recordings before / after fix and try to update unit / e2e test? And thank you for your contribution! |
@StyleT I've updated the PR and added screen recordings. However, adding tests is a bit tricky since this is a visual browser bug. Meaning, the |
Thanks so much for doing this! This bug has been the bane of our lives :) |
} | ||
} else if (nextLineBreak) { | ||
const element = document.createElement('br'); | ||
// Workaround for a bug in Safari where the cursor cannot be placed at the | ||
// end of a line. | ||
if (IS_SAFARI) { |
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.
This works great on desktop Safari, but not on mobile Safari. I think it needs to be IS_SAFARI || IS_IOS
in order to work on both.
Hi! Sorry for a long reply here... I see that this PR fixes the issue with the end-of-line cursor, however when image (as in your videos) is the first element on the line - now you can't place cursor at the start-of-line. Am I missing something? |
Any progress on this? This is a big issue for our project. Would be great if this get fixed! If there is anything I can do to help, please let me know! |
It was also a big issue for us, so we've been using the PR branch directly and so far haven't had any issue. |
The branch from @sodenn fork you mean? I installed it using Probably I'm doing something stupid, is this the same thing you used? |
So actually I ended up writing a script to use #!/usr/bin/env bash
set -e
if [ $# -ne 2 ]; then
echo "Given a branch and a repo, this builds the lexical package and records a patch in .yarn/patches."
echo "Usage: yarn patch:lexical <branch> <repo>"
exit 1
fi
BRANCH=$1
REPO=$2
# Make a temporary directory
TEMP_DIR=$(mktemp -d)
# Clone the fork
git clone -b $BRANCH --single-branch $REPO $TEMP_DIR
# Change directory to the temporary directory
cd $TEMP_DIR
# Set the required node version
echo "nodejs 20.11.0" > ./.tool-versions
# Build Lexical
npm install
npm run build-release
# Change back to the original directory
cd -
# Get all the lexical dependencies
DEPS=$(jq -r '.dependencies | keys | map(select(test("lexical")))' package.json)
readarray -t DEP_NAMES <<< "$(echo $DEPS | jq -r '.[]')"
# Find the folder for each dependency
for DEP in "${DEP_NAMES[@]}"; do
# Loop through all directories in $TEMP_DIR/packages
for DIR in $TEMP_DIR/packages/*; do
if [ -d "$DIR" ]; then # Ensure $dir is a directory
# Check if package.json exists in the directory
if [ -f "$DIR/package.json" ]; then
# Extract the name field from package.json
PACKAGE_NAME=$(jq -r '.name' "$DIR/package.json")
# Compare with the current dependency name
if [[ "$PACKAGE_NAME" == "$DEP" ]]; then
echo "Patching $PACKAGE_NAME (in $DIR)"
echo "PACKAGE_NAME: $PACKAGE_NAME"
echo "DIR: $DIR"
echo "TEMP_DIR: $TEMP_DIR"
echo "DEP: $DEP"
# Build the patch
PATCH_PATH=$(yarn patch $PACKAGE_NAME --json | jq -r '.path')
cp -R $DIR/dist/* $PATCH_PATH
# Determine if there is actually a difference
set +e
git diff --quiet --no-index $PATCH_PATH ./node_modules/$DEP
if [ $? -eq 0 ]; then
echo "No changes detected for $PACKAGE_NAME."
else
echo "Patching..."
yarn patch-commit -s $PATCH_PATH
echo "Generated patch for $PACKAGE_NAME."
fi
set -e
echo
fi
fi
fi
done
done |
Description
This PR addresses a bug in Safari where the cursor cannot be positioned at the end of a line when the line ends with a
contenteditable=false
element.Closes #4487
Of course, other text editors like ProseMirror have the same problem, and they work around it by adding an
<img>
element before the line break. This PR implements a similar approach and adds an<img>
element before the__lexicalLineBreak
element.Please note that there is a method called
isManagedLineBreak
inLexicalMutations.ts
that accesses__lexicalLineBreak
. It is unclear to me if the new<img>
element causes any issues in this context.❌ Before
✅ After
For further reference, see: