Skip to content

Add per-course xSpacing/ySpacing for scaffold reset layout - #115

Draft
pconrad with Copilot wants to merge 4 commits into
mainfrom
copilot/give-options-for-spacing
Draft

Add per-course xSpacing/ySpacing for scaffold reset layout#115
pconrad with Copilot wants to merge 4 commits into
mainfrom
copilot/give-options-for-spacing

Conversation

Copilot AI commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

POST /api/course/scaffold/reset laid out top-level concepts using hardcoded pixel spacing (350 horizontal, 300 vertical), with no way for instructors to adjust it.

Backend

  • Added xSpacing/ySpacing int columns to Course, defaulting to ConceptGraphService.MIN_HORIZONTAL_SEPARATION/VERTICAL_LEVEL_SEPARATION (350/300) — the same constants the layout algorithm already used
  • Added Liquibase migration 042-add-spacing-to-course.json backfilling existing courses with those defaults
  • Extended ConceptGraphService.reset(...) with an overload taking explicit xSpacing/ySpacing (existing overloads untouched, so default-spacing callers/tests are unaffected)
  • POST /api/course/scaffold/reset now reads the course's spacing and passes it through instead of using the constants directly
  • Added PUT /api/course/scaffold/spacing (courseId, xSpacing, ySpacing) to update these values, rejecting non-positive input
  • Exposed xSpacing/ySpacing on InstructorCourseView so the frontend can show current values

Frontend

  • ScaffoldTabComponent now fetches the course and renders editable X/Y Spacing fields with an "Update Spacing" button
  • Shows a green ✓ on successful update, an inline error alert otherwise
conceptGraphService.reset(
    topLevelConcepts,
    edges,
    priorXByConceptId,
    course.getXSpacing(),
    course.getYSpacing());

Copilot AI linked an issue Jul 14, 2026 that may be closed by this pull request
Copilot AI changed the title [WIP] Add xSpacing and ySpacing to Course table and endpoints Add per-course xSpacing/ySpacing for scaffold reset layout Jul 14, 2026
Copilot AI requested a review from pconrad July 14, 2026 18:32
@pconrad

pconrad commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

@copilot please resolve the merge conflicts.

…for-spacing

# Conflicts:
#	frontend/src/main/components/Courses/TabComponent/ScaffoldTabComponent.jsx
#	frontend/src/stories/components/Courses/TabComponents/ScaffoldTabComponent.stories.jsx
#	frontend/src/tests/components/Courses/TabComponent/ScaffoldTabComponent.test.jsx

Copilot AI commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please resolve the merge conflicts.

Resolved in 97d79b9 — merged origin/main. The reset-scaffold button/mutation removed from ScaffoldTabComponent by the "move to footer" PR was dropped from this branch too (it's dead code here now); kept the spacing fields and updated their story/test mocks accordingly. Verified full frontend (npm test, 664 tests) and backend (mvn test, 81 classes) suites pass.

@pconrad

pconrad commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

@copilot please take care of the ci/ci failures

Copilot AI commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please take care of the ci/ci failures

Fixed in 400d3... (pitest mutation coverage was 99%; a surviving mutant on the ySpacing <= 0 boundary check in updateScaffoldSpacing wasn't killed because the existing test used ySpacing=-1 instead of the boundary value 0). Added a test asserting ySpacing=0 is rejected — pitest now reports 100% mutation coverage. All 18 CI checks are green.

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.

Give options for spacing

2 participants