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: choose classroom page (distribute assessment - page 2) #461

Merged
merged 16 commits into from
Jul 22, 2023

Conversation

joyce-shi
Copy link
Contributor

@joyce-shi joyce-shi commented Jul 16, 2023

Notion ticket link

Choose Classroom page

Implementation description

  • Added the second page in the Distribute Assessment flow
    There are some design liberties I took here, such as empty states and hover behaviour, but I think we can sync up with design on this when we do a full review.

One key difference - I left the search bar off of this page. We don't implement a search bar for the Classrooms page, so I don't think it makes sense to implement one here. I think if we do want to, we should align to add this search bar to both views, so leaving it off this one for now.

@joyce-shi joyce-shi changed the base branch from staging to joyce/fix-classroom-card-spacing July 16, 2023 14:19
@github-actions
Copy link

github-actions bot commented Jul 16, 2023

Visit the preview URL for this PR (updated for commit 13b3429):

https://jump-math-staging--pr461-joyce-distribute-ass-31j17hfi.web.app

(expires Sat, 29 Jul 2023 19:32:37 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: c42d8d0d853b05885664a2dd73f8245f4333ae51

);
};

export default EmptyDistributeClassroomsMessage;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not specced by design but took some liberties here.

</LinkOverlay>
<ClassroomPopover />
</>
)}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same here, I didn't think the differences warranted a separate component, so just tried to make this one accommodate the minor differences from this view.

@joyce-shi joyce-shi marked this pull request as ready for review July 16, 2023 14:29
@joyce-shi joyce-shi requested a review from a team July 16, 2023 14:29
Base automatically changed from joyce/fix-classroom-card-spacing to staging July 16, 2023 17:58
Copy link
Collaborator

@carissa-tang carissa-tang left a comment

Choose a reason for hiding this comment

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

lookin good! left a few comments

Copy link
Collaborator

@carissa-tang carissa-tang left a comment

Choose a reason for hiding this comment

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

lgtm! one small comment, but approving to unblock you

Copy link
Collaborator

@carissa-tang carissa-tang left a comment

Choose a reason for hiding this comment

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

actually wait, it looks like the search count is wrong. looks like we're taking the length of the search keyword, not the results

image

@joyce-shi joyce-shi merged commit 2b14d57 into staging Jul 22, 2023
7 checks passed
@joyce-shi joyce-shi deleted the joyce/distribute-assessment-page-2 branch July 22, 2023 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants