Add draggable block support to list items #7640
Conversation
lequavis
requested review from
Fetz,
Sahejkm,
acywatson,
fantactuka,
ivailop7,
lilshady,
potatowagon,
takuyakanbr and
zurfyx
as code owners
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Hi @lequavis! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
facebook-github-bot
added
the
CLA Signed
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
|
This works much better than I would have expected, including transferring items between lists of different types. That said, the tests are failing, you need to run prettier to format. |
Thanks, I'm new to open-source/contributing to projects (and git for that matter), how do you want me to run prettier, and is that the only failing test? |
You can check the command in the package.json |
lequavis
force-pushed
the
draggable-list-items
branch
from
dafeddd to
eb46780
Compare
|
@ivailop7 I hope the prettier test now passes. Please LMK, thank you. |
| const anchorElementRect = anchorElem.getBoundingClientRect(); | ||
| const topLevelNodeKeys = getTopLevelNodeKeys(editor); | ||
| const anchorBounds = Rectangle.fromDOM(anchorElem); | ||
| const point = new Point(event.x, event.y); |
There was a problem hiding this comment.
shouldn't this account for the CSS zoom?
const zoom = calculateZoomLevel(elem);
|
It would also be great to update this PR to follow the pull request template. We use the PR titles and descriptions to put together the release notes and any PR that does not match the format adds work for maintainers and makes it hard for people to find things (e.g. it's not easy to know that this closes #7630, and merging this PR wouldn't cause an automation to close it). |
|
@lequavis are you planning on addressing the above comments and adding a test to wrap up this PR? |
4 participants
Hello, Lexical team!
I'm very excited to submit my first ever PR, and even happier it's for Lexical!
I've changed the
getBlockElementfrom targeting the base of the list (which could be aul, orol), to targeting the list items within, enhancing the overall functionality for lists.Thanks for reading, and I hope you like it! 😄