Skip to content

fix: Rename optimistic update in children containers - #90

Closed
ChrisChV wants to merge 7 commits into
chris/FAL-4165-fix-name-in-unit-sidebarfrom
chris/FAL-4165-fix-name-update
Closed

fix: Rename optimistic update in children containers#90
ChrisChV wants to merge 7 commits into
chris/FAL-4165-fix-name-in-unit-sidebarfrom
chris/FAL-4165-fix-name-update

Conversation

@ChrisChV

@ChrisChV ChrisChV commented Jun 6, 2025

Copy link
Copy Markdown
Member

Description

  • When you update the title of a unit/subsection in the subsection/section page, the text returns to the previous value for a while. This PR fix that issue
  • Which edX user roles will this change impact? "Course Author.

Supporting information

Testing instructions

  • Go to the library home of a library
  • Create one unit, one subsection and one section.
  • Add the unit to the subsection.
  • Add the subsection to the section.
  • On the section page, rename the subsection, verify that the optimistic update works.
  • On the subsection page, rename the unit, verify that the optimistic update works.

Other information

  • Fixing this issue results in an increase in requests, but the user is not affected for the use of placeHolderText

@pomegranited pomegranited left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 couple of lint issues but this is working well. I like the optimistic update behaviour, and would like to see it used everywhere.

I think we need a test or two though?

  • I tested this on my devstack with and without a running CMS, to make sure that the optimistic update reverts if it cannot be saved.
  • I read through the code
  • I checked for accessibility issues by using my keyboard to navigate
  • Includes documentation
  • User-facing strings are extracted for translation

Comment thread src/library-authoring/containers/ContainerEditableTitle.tsx Outdated
@codecov

codecov Bot commented Jun 11, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (chris/FAL-4165-fix-name-in-unit-sidebar@0e0c9ef). Learn more about missing BASE report.

Additional details and impacted files
@@                            Coverage Diff                             @@
##             chris/FAL-4165-fix-name-in-unit-sidebar      #90   +/-   ##
==========================================================================
  Coverage                                           ?   94.02%           
==========================================================================
  Files                                              ?     1163           
  Lines                                              ?    24421           
  Branches                                           ?     5194           
==========================================================================
  Hits                                               ?    22961           
  Misses                                             ?     1392           
  Partials                                           ?       68           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ChrisChV

ChrisChV commented Jun 12, 2025

Copy link
Copy Markdown
Member Author

Hi @bradenmacdonald, are you ok with these changes? I am planning to merge it with openedx#2100 and then merge that PR

Edit: I got confused, it's better to merge them separately because this one can't be backported to Teak

ChrisChV and others added 3 commits June 12, 2025 00:48
)

Fix the bug for show unit published name in sidebar on content picker.
Invalidates the query in the subsection page used to get the list of sections that contains the subsection
@bradenmacdonald

Copy link
Copy Markdown
Member

@ChrisChV Since this was targettng a branch on the open-craft fork, it doesn't need a CC review. I think you'll need to open a new PR if you want to merge it separately, and then I can review if you need.

@ChrisChV
ChrisChV force-pushed the chris/FAL-4165-fix-name-update branch from 81802ab to c5b1dd3 Compare June 12, 2025 17:52
@ChrisChV

Copy link
Copy Markdown
Member Author

Closed in flavor of openedx#2141

@ChrisChV ChrisChV closed this Jun 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants