-
-
Notifications
You must be signed in to change notification settings - Fork 462
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: File resize UX #1223
base: main
Are you sure you want to change the base?
fix: File resize UX #1223
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
I'm a bit concerned about this approach, see comment!
@@ -57,6 +57,17 @@ export const imageRender = ( | |||
block: BlockFromConfig<typeof imageBlockConfig, any, any>, | |||
editor: BlockNoteEditor<any, any, any> | |||
) => { | |||
// Immediately updates & re-renders the block if it's wider than the editor. | |||
if ( |
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.
My first feeling is I don't really like this approach:
- what if a user opens the document on a small screen, and later on a large screen?
- what if the user is in read-only mode?
@@ -372,20 +373,19 @@ NESTED BLOCKS | |||
height: 24px; | |||
} | |||
|
|||
[data-file-block] .bn-visual-media-wrapper { | |||
[data-file-block] .bn-resize-handles-wrapper { |
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.
make sure DO etc isn't using the old classnames. I suppose you didn't like the earlier name / just a cleanup right?
This PR fixes 2 bugs regarding file resize UX:
TODO:
Currently unit tests fail as something is going wrong when calling
updateBlock
on the initial render.getBlockFromPos
is throwing an error asgetPos()
returns 0.An alternative approach I got working was to check whether the default
previewWidth
is higher than the editor width when a file is initially added. However, that approach doesn't fix the case where the editor initial contents have a file that is too wide. So I think checking the width on update is the only solution that covers all cases.