-
-
Notifications
You must be signed in to change notification settings - Fork 10.2k
feat: bind text to shapes when pressing enter and support sticky notes 🎉 #4343
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… reaches to the top
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/excalidraw/excalidraw/8ouT19PSpDc98HhHEHWEGq7gmpGR |
…mensions in case it overflows
dwelle
reviewed
Dec 16, 2021
dwelle
reviewed
Dec 16, 2021
dwelle
reviewed
Dec 16, 2021
dwelle
reviewed
Dec 16, 2021
dwelle
reviewed
Dec 16, 2021
dwelle
reviewed
Dec 16, 2021
dwelle
approved these changes
Dec 16, 2021
Alright merging it 🎉 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
for #1428, #4239, #4267 ,#4270

NOTES
containerId
totext
element which refers to the container Id whenever its binded to a containerAddedusingboundTextElementId
to the container which refers to text element id to keep the mapping for text binded to the container - rename ?boundElements
for the mapping nowcontainerId
then don't include and similarly ingetElementsAtPosition
and few other places. But there would be cases where text element needs to be selected for example when exporting to png/svg, adding to library, for this I have added a flagincludeBoundText
to the utilgetSelectedElements
inselection.ts
originalText
in text element so that whenever container is resized I can run the word wrap algo on the original text astext
is what is rendered on canvas - I will check if we can just keeptext
asoriginalText
astext
is needed for rendering on canvas onlywordWrap
algo is an attempt to replicate the behaviour ofword-break: break-word
in css. It tries to break in words where ever possible and for longer strings it keeps concatenating until width of container is reached, have added few comments in the code as wellwhere array contains the width of the characters and array index is ascii code of character. Kept it an array so its easy to calculate min width of character which I am using in resize algo, we can keep a map as well for width too and cache the min width as well
The cache is in memory, check
charWidth
intextElement.tsx
wordWrap
is called , I check the diff between prev container width and new container width and if it exceeds min char width thenwordWrap
is called.TODOS / BUGS
the parent container should be selectable by clicking anywhere inside (irrespective of background fill), or at least when clicking on the text
text should be vertically centered (as you type. Right now it gets vertically centered only after you resize the parent container)
arrow binding to text should be disabled (it already seems to be for the most part, but there's still the indicator)
performance aside, there seems to be an infinite loop resulting in a process freeze when you make the width of the rectangle too small. Sometimes if the state change manages to be persisted to LS before the freeze, you can't fix the issue any other way than clearing the site data from settings. So we should guard against infinite loops no matter the performance of the size calculation.
diamond and ellipse text bounding box not correct in some cases:
Its better now with vertical align, and user can always resize if it goes out of bounding box. We can improve this later. Checked other platforms Miro (starts hiding text as text increases which is bad), Eraser (similar to ours), Gdrive (doesnt auto increase container)
word wrap issue which shows different words when editing vs submitted

Word wrap not working in safari - Use word-break: break word for better support and work in safari
word wrap algo not respecting spaces when word goes beyond width and next word start

Update word wrap algo to break words, right now its breaking at character. This would also help in improving perf overall and also when resizing
Calculate approx chars that could fit in a line in word wrap algo, this should significantly improve perf - probably not needed since caching works well, though I have added the util so we can use in future
Try canvasMeasureText to see if It could improve perf. Right now there is a slight diff of 1-2 pixels in word wrap algo, which could get fixed with this. This will reduce the time by ~10 times
For longer lines exceeding width, calculate the width of each char in order unless max width, this will optimize the perf as right now I am slicing string from end unless it fits the container which is very bad for small container where only one char could fit. This would make sure the number of runs don't exceed text length which is right now ~
((text.length * text.length -1) /2)
Start caching the width of each char to improve perf when resizing
Minimize the number of times
wrapText
is called when resizing to improve perf as its called very frequently even when width hasn't significantly changed which slows down resizing. We can check if width exceeded by atleastapproxMinWidth
before executing the algo. Notice the count of log "attempt to call wrap text(665)" vs "called wrap text(604)"- [ ] Use same div when calculating dimensions if available as appending/removing div on every calculation will have reflow / repaint issues. This will also improve perfThis does improve perf but not needed since now I am using canvas measureTextwhen copying (
cmd-c
oralt-drag
), the bound text is not copied, and gets bound to both the old and the new container at the same time. NOTE that the fix should also account for copy/pasting across instances (different browser...)The line height / height not getting updated once text properties are updated inside container
Center the text when properties updated
Add a hint to "Press enter to add text" when single element selected
When double clicked inside container, take the cursor to end of text same as what happens when enter is pressed
wen you rotate a rectangle, then release and start rotating again, the rectangle rotation changes (jumps) when you grab the handle:
Select container instead of text element when
escape
key is pressed and text is submittedAdd to library doesn't add the bounded text
Export to png/svg doesn't add bounded text
Fix grouping when container with text element included
Fix copy as svg/png
Make sure resize works when multiple elements selected
Make sure rotation works when multiple elements selected
Disable selecting text element when using selection tool
After rotation the container is not autoresizing
Fix word wrap algo to break properly length hasn't exceeded width of container
Add tests for
wordWrap
utilThere is a corner case where trailing spaces is not preserved as in algo its trimmed
deleting bound text (by editing it and confirming after removing all chars) should unset the
containerElement.boundTextElementId
. Otherwise the container still behaves as if it has bound text.Adding spaces behaves weirdly with chrome only, works fine in other browsers
- This is due to
text-align:center
which doesn't work well in chrome when adding spaces in chromecontinuous spaces don't wrap to next line in css which leads to inconsistencies when text is submitted
Support binding in images
Text gets submitted on blur so can't update color of text by updating hex code - existing bug
Fix the y coord when resizing from ne/nw/n
duplicating via
cmd/ctrl-D
doesn't handle bound text duplication properlyon resize via corner handles, we should either not allow resizing to the "negative" at all, or ensure it doesn't move the opposite edge position:
boundElements
not updated properly: duplicate objects accumulate on consecutive edits.wysiwyg editor progressively misaligns (on new lines added) when zoomed in/out. Below, 60% zoom:
This is an existing bug when height of text area goes beyond canvas height irrespective of zoom
only remove last trailing space which we have added when joining words
Fix alignment issues when text is right/left align and resizing
extra newline (?) or vertical offset not calculated correctly in some cases: