fix: prevent full iframe reload after xblock edition#2257
fix: prevent full iframe reload after xblock edition#2257bra-i-am wants to merge 9 commits intoopenedx:masterfrom
Conversation
|
Thanks for the pull request, @bra-i-am! 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. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2257 +/- ##
==========================================
- Coverage 94.83% 94.83% -0.01%
==========================================
Files 1231 1231
Lines 27627 27622 -5
Branches 6220 6053 -167
==========================================
- Hits 26199 26194 -5
- Misses 1357 1370 +13
+ Partials 71 58 -13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
739668d to
7cc6c1f
Compare
c5bffb2 to
e9f9d44
Compare
|
@bra-i-am Sorry for the delay here. I think this looks great. Could you please rebase it so we can make sure it's still working properly with the latest master, and then I'll test and approve it? |
021f797 to
55775e0
Compare
|
@bradenmacdonald, I’m so sorry for the delay, too. I’ve now merged the latest master changes here. I ran some tests, and everything seems to be working well. Please let me know your thoughts. |
|
I have merged #2600 and I believe this issue is now fixed. Please let me know if there are other changes from this PR that you'd like to merge, or if we can close this. |
Description
This PR addresses an issue that causes a full iframe reload after saving edited xblock data. To prevent the page reload, these changes remove the entire iframe reload, which was part of a previous implementation to display the
Publishbutton after editing an xblock. However, this PR uses a different approach to achieve that.This pull request refactors the handling of XBlock publish state in the course unit workflow, replacing local storage-based refresh triggers with a Redux-based state management approach. It introduces a new
setXBlockPublishStatereducer and related hook logic, ensuring that the UI updates correctly after discarding changes or saving edits. The changes also improve test coverage for these new behaviors.State management improvements:
setXBlockPublishStatereducer tosrc/course-unit/data/slice.jsto manage the published and hasChanges flags for XBlocks in Redux state.useCourseUnithook insrc/course-unit/hooks.jsxto include aresetXBlockPublishStatefunction, which dispatches the new action for resetting publish state, and removed the local storage event listener logic. [1] [2]CourseUnitandXBlockContainerIframecomponents to use and pass the newresetXBlockPublishStatefunction, ensuring that publish state is reset after discarding changes or completing XBlock editing. [1] [2] [3] [4] [5]Testing enhancements:
src/course-unit/data/slice.test.jsand for the hook logic insrc/course-unit/hooks.test.jsx, verifying correct state transitions and hook return values. [1] [2]src/course-unit/CourseUnit.test.jsxto confirm that the XBlock publish state is reset after discarding changes and the UI updates accordingly.Code cleanup:
Supporting information
This PR fixes point 1 in the issue #1893
Testing instructions