-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Upgrade client to React 18 (and upgrade dependencies). #6114
Conversation
This reverts commit e73d3bf.
src/avatar.html
Outdated
</script> | ||
</head> | ||
|
||
<body> | ||
<div id="support-root"></div> | ||
<div id="ui-root"></div> | ||
<div id="avatar-ui-root"></div> |
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.
Each html file has a unique ui-root id as React 18 complains if we createRoot on an id more than once.
Hii @nikk15 I have migrated the src/react-components/avatar-editor.js to React 18, I need to do some final sanity on it before a PR, I'll put the link to PR here ... I think regarding the formatting, those spaces are probably generated by the Typescript formatter, Having Prettier as default formatter should solve most of the problems |
This reverts commit 6480836.
const width = el.offsetWidth; | ||
const height = el.offsetHeight; | ||
const ratio = height / width; | ||
const scale = (CHAT_MESSAGE_TEXTURE_SIZE * Math.min(1.0, 1.0 / ratio)) / el.offsetWidth; |
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.
el.offsetWidth; could be "width"
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.
few comments here and in discord but no show stoppers. lgtm
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.
lgtm
I had hoped to upgrade Admin to React 18 also, but as there is a tangled web of packages relying on earlier versions of react, react-admin and material UI, I am leaving this for when we swap those components out for Lilypad.