Skip to content

Conversation

Arpan-206
Copy link
Member

@Arpan-206 Arpan-206 commented Oct 3, 2025

Checklist:

  • I've thoroughly self-reviewed my changes
  • I've added tests for my changes, unless they affect admin-only areas (or are otherwise not worth testing)
  • I've verified any visual changes using Percy (add a commit with [percy] in the message to trigger)

Note

Adds drag-and-drop reordering of enabled extensions, persists order via activation positions, and updates step list accordingly.

  • UI/UX (Configure Extensions modal):
    • Split extensions into enabledExtensions (sortable) and disabledExtensions with animated transitions in configure-extensions-modal/index.hbs.
    • Added drag handle and layout tweaks in extension-card.hbs; card now accepts @onToggle.
  • Component logic:
    • Moved activation toggle logic from extension-card.ts to parent index.ts (handleExtensionToggle), with optimistic state in card and onToggle callback.
    • Implemented handleSortableItemsReordered to update activation positions and persist order.
  • Data/model:
    • Added position attr and reorder collection action to course-extension-activation model.
    • StepListDefinition now groups/sorts extension steps by activation position when a stageList exists.
  • Mirage:
    • Create handler assigns incremental position; added POST /course-extension-activations/reorder to update positions and return sorted activations.
    • Seeding now sets initial position.
  • Tests:
    • New acceptance tests validate drag-and-drop reordering, persistence after modal close, and updated positions.

Written by Cursor Bugbot for commit a44f312. This will update automatically on new commits. Configure here.

cursor[bot]

This comment was marked as outdated.

Copy link

github-actions bot commented Oct 3, 2025

Test Results

675 tests  +1   627 ✅ +1   9m 33s ⏱️ -6s
  1 suites ±0    48 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit a44f312. ± Comparison against base commit 8b39b8d.

This pull request removes 3 and adds 4 tests. Note that renamed tests count towards both.
Chrome 140.0 ‑ Acceptance | course-page | complete-first-stage: retains state when navigating to other course page areas Chrome 140.0 ‑ Acceptance | course-page | complete-second-stage: can complete second stage Chrome 140.0 ‑ Acceptance | course-page | language-guides: can view language guides 
Chrome 140.0 ‑ Acceptance | course-page | complete-second-stage: can complete second stage when hints & solution are present Chrome 140.0 ‑ Acceptance | course-page | complete-second-stage: can complete second stage when solution & hints are not present Chrome 140.0 ‑ Acceptance | course-page | extensions | reorder-extensions: can reorder enabled extensions Chrome 140.0 ‑ Acceptance | course-page | extensions | reorder-extensions: reorder persists after closing and reopening modal 

♻️ This comment has been updated with latest results.

Copy link

codecov bot commented Oct 3, 2025

Codecov Report

❌ Patch coverage is 75.00000% with 11 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...ts/course-page/configure-extensions-modal/index.ts 72.97% 9 Missing and 1 partial ⚠️
app/utils/course-page-step-list.ts 80.00% 0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

Copy link

codecov bot commented Oct 3, 2025

Bundle Report

Changes will increase total bundle size by 2.47kB (0.01%) ⬆️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
client-array-push 38.62MB 2.47kB (0.01%) ⬆️

Affected Assets, Files, and Routes:

view changes for bundle: client-array-push

Assets Changed:

Asset Name Size Change Total Size Change (%)
assets/chunk.*.js 2.73kB 394.74kB 0.7%
assets/chunk.*.js -943 bytes 3.21MB -0.03%
assets/chunk.*.js -155 bytes 70.9kB -0.22%
assets/chunk.*.js 15 bytes 27.08kB 0.06%
assets/chunk.*.js -69 bytes 27.07kB -0.25%
assets/chunk.*.js -60 bytes 24.85kB -0.24%
assets/chunk.*.js -1.28kB 71.06kB -1.77%
assets/chunk.*.js 2.09kB 24.91kB 9.16% ⚠️
assets/chunk.*.js 12.81kB 72.34kB 21.52% ⚠️
assets/chunk.*.js 2.02kB 59.53kB 3.5%
assets/chunk.*.js 3.58kB 57.51kB 6.65% ⚠️
assets/chunk.*.js 693 bytes 22.82kB 3.13%
assets/chunk.*.js 873 bytes 22.13kB 4.11%
assets/chunk.*.js 3.95kB 21.25kB 22.83% ⚠️
assets/chunk.*.js 2.04kB 17.3kB 13.4% ⚠️
assets/chunk.*.js 6.42kB 53.93kB 13.5% ⚠️
assets/chunk.*.js 40.56kB 47.51kB 583.82% ⚠️
assets/chunk.*.js -79.82kB 15.26kB -83.95%
assets/chunk.*.js (New) 6.95kB 6.95kB 100.0% 🚀
assets/chunk.*.css 65 bytes 247.24kB 0.03%

Files in assets/chunk.*.js:

  • ./models/course-extension-activation.ts → Total Size: 2.15kB

  • ./utils/course-page-step-list.ts → Total Size: 5.87kB

Files in assets/chunk.*.css:

  • ./utils/course-page-step-list.ts → Total Size: 5.87kB

  • ./models/course-extension-activation.ts → Total Size: 2.15kB

@Arpan-206 Arpan-206 force-pushed the arpan/cc-1939-change-extensions-configuration-ui branch from a763828 to f554778 Compare October 6, 2025 19:42
cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

@rohitpaulk
Copy link
Member

  • Disabled cards have a different padding distance between them than enabled cards:
Screenshot 2025-10-16 at 11 55 21
  • Disabling experience is jarring - the card initially stays in that position in the disabled state and then disappears:
Screen.Recording.2025-10-16.at.11.56.25.mov

Ideally this'd look like a list animation, check how we handle leaderboard item movement on the CourseLeaderboard (sidebar), that's smooth

  • Drag handle and text don't look vertically aligned
Screenshot 2025-10-16 at 11 57 05

Only looking at UX for now, will comment on code once this feels shippable

Copy link
Member

@rohitpaulk rohitpaulk left a comment

Choose a reason for hiding this comment

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

Comment added

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

@rohitpaulk
Copy link
Member

@Arpan-206 what exactly was fixed here since the last review other than the first point (padding distance)?

Disabling is still jarring - card stays for a while and then poof disappears:

Screen.Recording.2025-10-19.at.15.38.38.mov

Handles are still not vertically aligned with text:

Screenshot 2025-10-19 at 15 39 53
Copy link
Member

@rohitpaulk rohitpaulk left a comment

Choose a reason for hiding this comment

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

Comment added

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants