Repository navigation
Conversation
|
Thanks for the pull request, @Kelketek! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
| @@ -0,0 +1,8 @@ | |||
| import { type XBlockConfig } from "@opencraft/xblock-react-utils"; | |||
There was a problem hiding this comment.
@Kelketek I think we should upstream the utils package to namespace it under @openedx or make them a part of the frontend itself.
There was a problem hiding this comment.
@tecoholic Agreed-- that's part of the original proposal. It's a bit chicken-and-egg here-- I'm expecting we'll need a follow-up post-contribution to update the references once we move the repo in to the @openedx namespace, but I wanted to make sure that we had a working end-to-end setup that was properly inclusive before pushing that.
| @@ -0,0 +1,27 @@ | |||
| import type { ReactNode } from "react"; | |||
| import { RuntimeContext } from "@opencraft/xblock-react-utils"; | |||
There was a problem hiding this comment.
This is related to the lib/context.ts. If the utils package already ships a RuntimeContext, could we either merge the config into it? Or just put the context and useXBlockConfig hook into the utils?
| @@ -0,0 +1,27 @@ | |||
| import type { ReactNode } from "react"; | |||
| import { RuntimeContext } from "@opencraft/xblock-react-utils"; | |||
| import { mockRuntime, render } from "@opencraft/xblock-react-utils/testing"; | |||
There was a problem hiding this comment.
Similarly, could we upstream the testing utils?
There was a problem hiding this comment.
This would be part of moving this package to upstream, yes.
| "oxfmt": "^0.73.0", | ||
| "oxlint": "^1.88.0", | ||
| "react-intl": "^6.8.9", | ||
| "typescript": "~5.9.3", |
There was a problem hiding this comment.
@Kelketek Typescript 7.0 was released in July this year, which offers upward of 10x speed improvements. Can we use it here?
| type XBlockRuntime, | ||
| } from "@opencraft/xblock-react-utils"; | ||
| import type { {{cookiecutter.class_name}}Fields } from "@/types.ts"; | ||
| import styles from "./style.scss?inline"; |
There was a problem hiding this comment.
A comment here about why ?inline here is necessary and if all styles should be imported this way would be helpful.
| saveStudioBlock<{{cookiecutter.class_name}}Fields>({ | ||
| saveUrl: studioSaveUrl, | ||
| data: saveData, | ||
| runtime, | ||
| errorMessage: "Failed to save configuration", | ||
| }); | ||
| }; |
There was a problem hiding this comment.
I am conflicted by this function being a util. While working on the JSON import, I had to re-implement the "save" functionality with some custom logic and realized this utility function is useful only in the narrow context. If we were to replace this with $.post(runtime.handlerUrl(element, studio_save), saveData)... instead, we will be able to document the use of Python handler function inside React and provide an example for developers - at the expense of being concise.
| createXBlockRoot({ runtime, element, config: context.config, styles })( | ||
| <XBlockConfigContext.Provider value={context.config}> | ||
| <App {...context.data} /> | ||
| </XBlockConfigContext.Provider>, | ||
| ); |
There was a problem hiding this comment.
Just an extension of the previous comments about contexts - should we just merge the Runtime and Config contexts as single context and just make it
createXBlockRoot({...})(<App {...data} />)
| minimumReleaseAgeExclude: | ||
| - "@opencraft/xblock-react-utils@0.1.1" |
There was a problem hiding this comment.
This is okay for testing. Ideally we don't have anything here? Or we are make this package a target for hijackers.
There was a problem hiding this comment.
It probably isn't needed anymore since 0.1.1 is now old enough. However, you can't replace a version on NPM these days, so as long as the entry is known good. So, this isn't introducing a vulnerability.
Per the NPM docs:
"Once a package is published with a given name and version, that specific name and version combination can never be used again, even if it is removed with npm unpublish."
If we end up updating the package as part of this work we'll need to bump the version here, though :)
| /* | ||
| * The base styles of cms set html { font-size: 62.5% } to get the base font size of 1rem = 10px. | ||
| * But Paragon's CSS Variables define the text sizes in "rem" units with the expectation of 1rem = 16px. | ||
| * To fix this, we set html { font-size: 100% } to make 1rem = 16px, matching the Paragons CSS Variables. | ||
| * | ||
| * Note that this only affects things within the shadow root created for the XBlock, so it won't affect | ||
| * things outside of the XBlock editor. | ||
| */ | ||
| html { | ||
| font-size: 100%; | ||
| } |
There was a problem hiding this comment.
This should only be in the Python package's static/css/studio.css right? Possible duplicate?
There was a problem hiding this comment.
Yep, that's right-- I had placed it here at one point thinking we might be able to avoid that, but it didn't seem so and I forgot to remove it. I'll clear it out.
| description = "{{cookiecutter.short_name}} XBlock" # TODO: write a better description. | ||
| readme = "README.md" | ||
| # license = "UNKNOWN" # TODO: choose a license: 'AGPL-3.0-or-later' and 'Apache-2.0' are popular. | ||
| # Version used by the platform as of Verawood. Lower this if you need to support older versions, |
| @@ -0,0 +1,59 @@ | |||
| import type { SaveStudioBlockParams } from "@opencraft/xblock-react-utils"; | |||
There was a problem hiding this comment.
@tecoholic I'm still working on your notes, but I wanted to call your attention to this file I've added.
This contains a revised version of saveStudioBlock as well as a function which makes jQuery promise-based. Please give these a look over and if they look good I'll put them in a PR to xblock-react-utils.
There was a problem hiding this comment.
I like the revised function, as it takes the saveUrl as a param and can be used across all the XBlock handlers now.
|
|
||
| export const App = (data: {{cookiecutter.class_name}}Fields) => { | ||
| const { runtime } = useContext(RuntimeContext); | ||
| const studioSaveUrl = useHandlerUrl("studio_save"); |
There was a problem hiding this comment.
Oh! Wow. There exists a useHandlerUrl hook? Nice.
| } | ||
|
|
||
| const promise = new Promise<T>((resolve, reject) => { | ||
| $.ajax(resolvedSettings as JQueryAjaxSettings).done(resolve).fail(reject); |
Description
This pull request adds React and Paragon as well as modern build tooling to the XBlock template. This allows XBlock developers to get started with a practical, end-to-end example of React-based XBlock development, and allows them to begin using Paragon and design tokens in their blocks today.
Supporting information
https://openedx.atlassian.net/wiki/spaces/OEPM/pages/6574833665/Proposal+Update+XBlock+Examples+Templates+and+Documentation+to+be+React+and+Paragon-based
Internal ticket: https://tasks.opencraft.com/browse/BB-11326
Testing instructions
uv syncuv run ./bin/workbench-make-xblock