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.

Sign up for GitHub

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

Jump to bottom

Course Credit Filter #668

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Course Credit Filter #668

wants to merge 8 commits into from

Conversation

Qtssuisz
Copy link

closes #653

filtering classes with their corresponding credit

image

@chavab0809 chavab0809 self-requested a review March 17, 2023 21:34
chavab0809
chavab0809 previously approved these changes Mar 17, 2023
View reviewed changes
Copy link

@chavab0809 chavab0809 left a comment

Choose a reason for hiding this comment

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

Everything looks good I just ran it on the local hosts and did some tests and they all seemed fine. The only thing I would recommend is formatting the check box next to the credit options to be a little more spaced out. But other than that it seems good!

@Qtssuisz
Copy link
Author

chavab0809

I couldn't find a perfect way for the checkboxes layout with space between the checkboxes and their labels but I swapped their position

I hope this will look better

image

@DarknessUnder DarknessUnder self-requested a review March 21, 2023 17:58
@ajwadcs ajwadcs self-requested a review March 24, 2023 20:22
ajwadcs
ajwadcs approved these changes Mar 24, 2023
View reviewed changes
Copy link

@ajwadcs ajwadcs left a comment

Choose a reason for hiding this comment

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

Runs fine and looks pretty good for an initial design. Maybe could turn the credit filter into a dropdown menu instead of checking boxes, but just a suggestion.

@mlammon11 mlammon11 self-requested a review March 25, 2023 21:13
mlammon11
mlammon11 suggested changes Mar 25, 2023
View reviewed changes
Copy link
Contributor

@mlammon11 mlammon11 left a comment

Choose a reason for hiding this comment

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

Feature works as intended and is a great addition to improving user experience. However the spacing seems a bit off, if you could fix the spacing or maybe make the buttons into a drop down, then it would be great.

@DarknessUnder DarknessUnder changed the title Qtssuisz Course Credit Filter Apr 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mlammon11 mlammon11 mlammon11 requested changes

@DarknessUnder DarknessUnder DarknessUnder approved these changes

@ajwadcs ajwadcs ajwadcs approved these changes

@zhangn7 zhangn7 zhangn7 approved these changes

@chavab0809 chavab0809 chavab0809 left review comments

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Feature Request — filtering classes with their corresponding credit
6 participants
@Qtssuisz @DarknessUnder @ajwadcs @mlammon11 @zhangn7 @chavab0809