Skip to content

Let users reorder classes in added courses pane#1554

Draft
Choollol wants to merge 27 commits intomainfrom
AA-903-reorder-added-courses
Draft

Let users reorder classes in added courses pane#1554
Choollol wants to merge 27 commits intomainfrom
AA-903-reorder-added-courses

Conversation

@Choollol
Copy link
Copy Markdown
Contributor

@Choollol Choollol commented Mar 7, 2026

Summary

Allows reordering of classes within the added courses pane.

Test Plan

  1. Ensure styling is the same for added courses pane and other places that use affected components, like search results
  2. Ensure drag and drop works
  3. Ensure order of courses is saved after reordering

Issues

Closes #903

@Choollol Choollol marked this pull request as ready for review March 11, 2026 23:35
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

3 issues found across 12 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/antalmanac/src/stores/Schedules.ts">

<violation number="1" location="apps/antalmanac/src/stores/Schedules.ts:203">
P2: Guard against missing course IDs before calling moveArrayElements; otherwise a -1 index will move the last course instead of the intended one.</violation>
</file>

<file name="apps/antalmanac/src/stores/AppStore.ts">

<violation number="1" location="apps/antalmanac/src/stores/AppStore.ts:391">
P2: Emit an added-courses change event after reordering so listeners update. Right now the reorder mutation doesn’t notify subscribers.</violation>
</file>

<file name="apps/antalmanac/src/components/drag-and-drop/SortableList.tsx">

<violation number="1" location="apps/antalmanac/src/components/drag-and-drop/SortableList.tsx:63">
P3: `sx` supports arrays in MUI, but spreading it into an object assumes it is a plain style object. If a caller passes an sx array, those styles won’t apply correctly. Normalize `sx` to an array before merging.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread apps/antalmanac/src/stores/Schedules.ts
Comment thread apps/antalmanac/src/stores/AppStore.ts
Comment thread apps/antalmanac/src/components/drag-and-drop/SortableList.tsx Outdated
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/antalmanac/src/components/drag-and-drop/SortableList.tsx">

<violation number="1" location="apps/antalmanac/src/components/drag-and-drop/SortableList.tsx:6">
P2: Avoid importing from `@mui/x-date-pickers/internals`; it is a private API and may break on MUI X updates.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

import { SortableContext, arrayMove, sortableKeyboardCoordinates } from '@dnd-kit/sortable';
import { List } from '@mui/material';
import { List, SxProps } from '@mui/material';
import { mergeSx } from '@mui/x-date-pickers/internals';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: Avoid importing from @mui/x-date-pickers/internals; it is a private API and may break on MUI X updates.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/antalmanac/src/components/drag-and-drop/SortableList.tsx, line 6:

<comment>Avoid importing from `@mui/x-date-pickers/internals`; it is a private API and may break on MUI X updates.</comment>

<file context>
@@ -3,6 +3,7 @@ import type { Active, UniqueIdentifier } from '@dnd-kit/core';
 import { restrictToVerticalAxis } from '@dnd-kit/modifiers';
 import { SortableContext, arrayMove, sortableKeyboardCoordinates } from '@dnd-kit/sortable';
 import { List, SxProps } from '@mui/material';
+import { mergeSx } from '@mui/x-date-pickers/internals';
 import type { ReactNode } from 'react';
 import { Fragment, useMemo, useState } from 'react';
</file context>

Copy link
Copy Markdown
Collaborator

@alexespejo alexespejo left a comment

Choose a reason for hiding this comment

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

Sorry for the delay I'm surveying some feedback – and this PR lwky keeps getting lost in my mind when I scroll up and down the PR board. I'm thinking that after we do #1643 and #1630 there will be new space to reorder some of the buttons and change up the design. Lmk what you think. Otherwise feature works great

marginTop: '4px',
}}
>
{sortable ? (
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think it would make sense to have the "grabbing hand" cursor on hover

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Getting the "grab" cursor is easy, but making it "grabbing" when dragging is harder. Do you think it would be worth it to get the "grabbing" cursor too, or leave it at just "grab"?

@Choollol
Copy link
Copy Markdown
Contributor Author

Sorry for the delay I'm surveying some feedback – and this PR lwky keeps getting lost in my mind when I scroll up and down the PR board. I'm thinking that after we do #1643 and #1630 there will be new space to reorder some of the buttons and change up the design. Lmk what you think. Otherwise feature works great

I think holding off until after the redesigns makes sense!

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 3 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/antalmanac/src/components/drag-and-drop/SortableList.tsx">

<violation number="1" location="apps/antalmanac/src/components/drag-and-drop/SortableList.tsx:51">
P2: Resetting `document.body.style.cursor` only in `onDragEnd` leaves a stuck global cursor when drag is canceled. Clear the cursor in `onDragCancel` as well.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread apps/antalmanac/src/components/drag-and-drop/SortableList.tsx
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.

Reorder classes in added courses pane

2 participants