Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add grade buttons to scheduled item cards #1209

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jpetto
Copy link
Collaborator

@jpetto jpetto commented Jul 9, 2024

Goal

add grade buttons to scheduled item cards.

Todos

  • push the backend PR to dev and test the full flow of data to snowplow
  • add tests for the new buttons?

Reference

Tickets:

@jpetto jpetto requested a review from a team as a code owner July 9, 2024 22:36
@jpetto jpetto marked this pull request as draft July 9, 2024 22:37
undefined,
refetch,
);
};
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

is this a good place to define this function?

the next PR for this will be to add the grade field to the item edit modal on all pages - which makes me think this function should be abstracted at a higher level, but i don't see an existing pattern for such a thing.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think if you look at the existing pattern & function above this, moveItemToBottom, you can leave it here

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's fine. This component is where a big chunk of SchedulePage was moved out to. The plan is to split it further into testable components some time in the future 🤔

@jpetto jpetto force-pushed the MC-1156-add-grades-to-schedule-view branch from ec9376a to a7c59b0 Compare July 9, 2024 22:42
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