Skip to content
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

feat(whiteboard): add tldraw support #205

Merged
merged 3 commits into from
Jul 9, 2022

Conversation

germanocaumo
Copy link
Contributor

Uses the events and files from bigbluebutton/bigbluebutton#15244

Closes #191.

23-06-2022-39wk0_Trim.mp4

- Had to remove xml2js to te able to update react-scripts to 5.0
-- Replaced by https://goessner.net/download/prj/jsonxml/json2xml.js (with some small modifications)
- Retro-compatible with old recordings
@pedrobmarin
Copy link
Collaborator

It looks great, @germanocaumo ! I'll try to take a look this weekend

src/utils/builder.js Outdated Show resolved Hide resolved
@pedrobmarin
Copy link
Collaborator

Maybe it would be a good opportunity to improve the tests over data's build stage. We could add some xml/json/... as mock data and make sure we keep the same result from before and after these changes.

- RenderedBounds from tldrawAPI was not updating right away when switching from deskshare, use the size directly.
- Refactor code a bit following suggestions from review
@pedrobmarin
Copy link
Collaborator

Another thing that is nice to do when a new whole component is included that has it's state controlled by the customs hooks is to check CPU usage while playing the recording. If you could compare CPU between tldraw and the current presentation (using similar annotations and stuff) we can anticipate some issues users might have.

@germanocaumo
Copy link
Contributor Author

Will let the remaining suggestions from pedro as issues for now:
#208
#207

And I think we can merge this so people can start using with bbb 2.6 alpha and help find more issues.

@pedrobmarin
Copy link
Collaborator

Will let the remaining suggestions from pedro as issues for now: #208 #207

And I think we can merge this so people can start using with bbb 2.6 alpha and help find more issues.

Yeah. I think this might be ok but BBB had an issue on it's deploy. The different versions of BBB weren't building from the playback tags. @antobinary could you check on that?
Basically we can't push these changes if they will trigger updates for 2.4 and 2.5...

@pedrobmarin
Copy link
Collaborator

I'll merge this to develop and create a new tag over the develop branch. Then I'll send a PR for BBB 2.6.x updating the playback source tag.

Here's some extra details on my main concern described in the previous comment: I believe BBB 2.4 and bellow were monitoring the master branch changes to generate new packages. This is not ideal for many reasons. At this repository, tags are usually generated from the master branch, so we would find ourselves in a though position trying to test this feature with 2.6 alpha.
To workaround this issue we can just avoid pushing develop to master for a while.

@pedrobmarin pedrobmarin requested review from pedrobmarin and removed request for pedrobmarin July 9, 2022 19:48
@pedrobmarin pedrobmarin merged commit 32564dd into bigbluebutton:develop Jul 9, 2022
pedrobmarin added a commit to bigbluebutton/bigbluebutton that referenced this pull request Jul 9, 2022
## What's Changed
* feat(playback): move react-scripts to devDependencies by @frankemax in bigbluebutton/bbb-playback#199
* Add regional fallbacks for all languages by @MBM1607 in bigbluebutton/bbb-playback#201
* Replace ReactDOM.render with createRoot by @MBM1607 in bigbluebutton/bbb-playback#200
* chore: locales from upstream by @pedrobmarin in bigbluebutton/bbb-playback#204
* feat(whiteboard): add tldraw support by @germanocaumo in bigbluebutton/bbb-playback#205

## New Contributors
* @frankemax made their first contribution in bigbluebutton/bbb-playback#199
* @MBM1607 made their first contribution in bigbluebutton/bbb-playback#201

**Full Changelog**: bigbluebutton/bbb-playback@v4.0.0...v5.0.0-alpha.1
@germanocaumo
Copy link
Contributor Author

Thanks a lot @pedrobmarin!!! ✨

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace xml2js with a frontend-compatible XML parser
2 participants