-
Notifications
You must be signed in to change notification settings - Fork 36
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
1605/replace render #1606
1605/replace render #1606
Conversation
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.
All affected pages are updated, this works just fine. Nice job.
Two small nit-picks:
- Your import statements
import {createRoot} from "react-dom/client"
should use single quotes, not double quotes. - See the comment I left in
detail-carousel.jsx
.
Once those are corrected, this is ready to merge.
src/components/detail-carousel.jsx
Outdated
@@ -248,6 +248,9 @@ const DetailCarousel = ({widget, widgetHeight=''}) => { | |||
|
|||
const snapToImage = (fast=false) => { | |||
const _pics = picScrollerRef.current | |||
if(!_pics){ |
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.
Single line expressions in a function can be condensed to:
if ( ! _pics) return false
If using return
to short-circuit a function, prefer return false
.
With my latest commit to this branch, I have updated all the pages to use single quotes and I returned false in the |
You updated double -> single quotes for
I can merge this as soon as you update double -> single quotes for the |
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.
In this pull request I went ahead and updated React to the latest version as well as using replacing the deprecated function
ReactDOM.render()
with the newcreateRoot()
function as recommend by the React docs .Details:
As the React docs mentions, the new way of making root components are to get the element and call
createRoot
to create a React root for displaying content inside a browser DOM element. After creating the root element, React needs it to callroot.render()
to render components inside it.A minor null check was needed for
src/components/detail-carousel.jsx
. The order in which react renders components is different now when usingcreateRoot
so without the null check clicking on a widget from the widget catalog would show a blank white page with an error saying that_pics
is null in the functionsnapToImage
.This pull request handles and replaces all the old files in the codebase that used
ReactDOM.render()
with the newcreateRoot()
function.