[lexical-playground] Bug Fix: Allow scrolling if the table cell content overflows #6966
Conversation
…l content overflows, and inheriting the same behavior for the nested <p> tag.
Parasaran-Python
requested review from
zurfyx,
fantactuka,
acywatson,
Fetz,
ivailop7,
Sahejkm and
potatowagon
as code owners
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
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 cla@meta.com. Thanks! |
size-limit report 📦
|
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! |
| @@ -15,6 +15,7 @@ | |||
| .PlaygroundEditorTheme__paragraph { | |||
| margin: 0; | |||
| position: relative; | |||
| overflow: inherit; | |||
There was a problem hiding this comment.
In what situations would it be useful for this nested p tag to scroll separately?
There was a problem hiding this comment.
without this the image doesnt scroll, the p tag seems to be blocking the scroll of the image as the image is eventually inside the p.
I could have very well applied the overflow prop to the p tag directly, but i felt it made more sense to apply it to td tag, and inherit to the child p tag
There was a problem hiding this comment.
Yes, the td tag needs it for sure. If remove this change with dev tools in a locally copy I can still scroll the cell horizontally. The only difference from here appears to be that the image gets clipped by the td padding on the right when it is enabled.
Screen.Recording.2024-12-14.at.08.28.23.mov
There was a problem hiding this comment.
Works for me as well in chrome, didnt work for me in firefox, hence I had to add it.
Seems like this is a browser specific thing, I hope even you would face the same issue in a firefox instance.
Not sure how the behavior would be on safari!
PS: May be chrome implicitly inherits the parent CSS and firefox doesnt! Not sure.
There was a problem hiding this comment.
This does look like a firefox-specific issue, which is "fixed" but currently behind a feature flag that defaults to false https://bugzilla.mozilla.org/show_bug.cgi?id=1904159
There was a problem hiding this comment.
That's a great example of why cross-browser testing is so important! 😅
Regarding this PR, it might be safer to go ahead with this change, unless there's a risk of any potential breakage. What are your thoughts?
There was a problem hiding this comment.
Sure let me do that.
There was a problem hiding this comment.
Now that I've had a bit more time to think about this, let's move this to a separate CSS rule that specifically targets any direct descendant of the table cell. This both covers the intent of the change and is more correct (will work for other top-level blocks, not just ParagraphNode).
/*
A firefox workaround to allow scrolling of overflowing table cell
ref: https://bugzilla.mozilla.org/show_bug.cgi?id=1904159
*/
.PlaygroundEditorTheme__tableCell > * {
overflow: inherit;
}There was a problem hiding this comment.
Yes, makes sense, will try this.
There was a problem hiding this comment.
This is done.
etrepum
changed the title
|
Comment has been added @etrepum requesting you to re-review. 🙂 |
fix 6931: CSS changes in the playground to allow scrolling if the cell content overflows, and inheriting the same behavior for the nested
<p>tag.Bug Fix
Description
Describe the changes in this pull request
Made changes to the playground CSS to allow scroll of table content, since there's a nested p tag, have added scroll:inherit to it
Closes #6931
Test plan
Try scrolling of various contents inside playground table's cells
Before
Insert relevant screenshots/recordings/automated-tests
After
bandicam.2024-12-14.16-49-17-307.mp4
Insert relevant screenshots/recordings/automated-tests