fix: do not reload multiple tabs on block save #2600
Conversation
|
Thanks for the pull request, @asadali145! 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. Where 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. |
openedx-webhooks
added
the
open-source-contribution
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2600 +/- ##
========================================
Coverage 94.82% 94.82%
========================================
Files 1231 1231
Lines 27629 27629
Branches 6222 6055 -167
========================================
Hits 26199 26199
Misses 1372 1372
Partials 58 58 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
bradenmacdonald
left a comment
bradenmacdonald
left a comment
There was a problem hiding this comment.
Looks good to me. Just one request before I merge:
| dispatch(actions.app.setSaveResponse(response)); | ||
| const parsedData = JSON.parse(response.config.data); | ||
| if (parsedData?.has_changes) { | ||
| if (parsedData?.has_changes || parsedData?.category === 'video') { |
There was a problem hiding this comment.
How about something like this? Or if you want to keep it this way, please add a comment explaining that video doesn't properly include the has_changes field.
| if (parsedData?.has_changes || parsedData?.category === 'video') { | |
| if (parsedData?.has_changes || !("has_changes" in parsedData)) { |
There was a problem hiding this comment.
Thanks! Looks good and incorporated in the PR.
|
@asadali145 Also, how does this compare to #2257 ? |
asadali145
force-pushed
the
asad/fix-page-reload-on-block-save
branch
from
d815217 to
4cb4d8e
Compare
@bradenmacdonald Thanks for reviewing the PR. The referenced PR looks good to me. I tried to fix the things following the existing approach with minimal changes. The referenced PR also fixes the issues that I tried to fix in this PR. I was testing that and found a small bug that when we open the problem editor and do not make any changes cancel it then it enables the publish button. Otherwise, it looks good to me. |
3 participants
Description
This PR fixes the below issues:
BEFORE:
Screen.Recording.2025-11-04.at.12.51.08.PM.mov
AFTER:
Screen.Recording.2025-11-04.at.1.08.12.PM.mov
BEFORE:
Screen.Recording.2025-11-04.at.12.52.09.PM.mov
AFTER:
Screen.Recording.2025-11-04.at.1.08.52.PM.mov
BEFORE:
Screen.Recording.2025-11-04.at.12.53.57.PM.mov
AFTER:
Screen.Recording.2025-11-04.at.1.09.31.PM.mov
Testing instructions