- 
                Notifications
    You must be signed in to change notification settings 
- Fork 7.5k
fix: migrate to new Excalidraw v0.18.0 #16392
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
base: master
Are you sure you want to change the base?
Conversation
| Hi, thanks for your contribution! | 
        
          
                package.json
              
                Outdated
          
        
      | "react-youtube": "10.1.0", | ||
| "redux": "4.0.4", | ||
| "redux-thunk": "2.4.1", | ||
| "roughjs": "^4.6.4", | 
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.
Can we add a peer dependency for this in our package, rather than require ie be here?
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.
looking into this
| import React, { useCallback, useEffect, useRef } from 'react'; | ||
| import { WithTranslation } from 'react-i18next'; | ||
| import { useSelector } from 'react-redux'; | ||
| import "@jitsi/excalidraw/index.css"; | 
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.
Why sis this needed now?
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.
they have updated their implementation, figured this out after reading their docs, mentioned under excalidraw Wrapper, https://docs.excalidraw.com/docs/@excalidraw/excalidraw/integration
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.
For some reason in the Module Bundler example it doesn't import the CSS. Can you check if it works without it?
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.
No Saul, Whiteboard is not working, If I am removing this
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.
Can we import it in ExcalidrawApp instead?
        
          
                webpack.config.js
              
                Outdated
          
        
      | }, { | ||
| test: /\.m?js$/, | ||
| resolve: { | ||
| fullySpecified: false | 
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.
What does this do?
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 makes module imports more flexible and prevents build errors from incomplete import path, which are present in the excalidraw codebase
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.
Is it possible to apply that to the Excalidraw package only?
Does disabling chunking on the build help in any way?
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.
Removed roughjs from the project dependencies and also removed this part of the code
Added roughjs as a dependency inside the excalidraw package, and configured an alias for roughjs in the webpack file, since Excalidraw was using incomplete import paths from roughjs folders
| theme: 'light', | ||
| UIOptions: WHITEBOARD_UI_OPTIONS | ||
| UIOptions: { | ||
| ...WHITEBOARD_UI_OPTIONS | 
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.
Unnecessary change.
| import React, { useCallback, useEffect, useRef } from 'react'; | ||
| import { WithTranslation } from 'react-i18next'; | ||
| import { useSelector } from 'react-redux'; | ||
| import "@jitsi/excalidraw/index.css"; | 
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.
Can we import it in ExcalidrawApp instead?
| This is a really exciting update. Any idea on if it can be completed ? Happy to help if needed. | 
| We are almost there, it's a GSoC project. | 
| show error:  | 
| Hey @jiyeyuran, work is in progress for the new excalidraw, in the latest version Firebase will be replaced with the S3 compatible backend for storage | 
| 
 OK,I thought the latest tag 0.18.1 is stable.It seems that jitsi/excalidraw needs some options to let users provide the getFiles and saveFiles methods for FileManager. | 
This PR upgrades the whiteboard integration in Jitsi Meet to use the latest Excalidraw release jitsi/excalidraw#8