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

change edit courses frontend #777

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

Conversation

swang235
Copy link
Contributor

@swang235 swang235 commented Nov 13, 2023

Summary

Changing the frontend of the edit classes page. Updating styling to be more aligned with the rest of the website.

previous design:
image

new design:
image

Test Plan

  • run yarn start and confirm that search bar functions properly.

Breaking Changes

None

  • I have updated the documentation accordingly.
  • My PR adds a @ts-ignore

@swang235 swang235 requested a review from a team as a code owner November 13, 2023 04:19
@dti-github-bot
Copy link
Member

dti-github-bot commented Nov 13, 2023

[diff-counting] Significant lines: 285.

Copy link
Contributor

@rgu0114 rgu0114 left a comment

Choose a reason for hiding this comment

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

Great work with this so far Sophie, I like the documentation you included and how you maintained the styling in accordance with the rest of QMI. I think one thing that would be helpful, though, is if you included a couple screenshots of your changes so it's easier to see which frontend updates you made

moved selected classes to middle of screen, updated role position, moved heading and changed background
need to do- fix courseText format to slightly lower, add search bar to top right of screen
@burninglilies
Copy link
Contributor

Looks great so far! Just remember to check for lint errors. Your only error rn is an indentation one so should be a v small fix

@swang235 swang235 changed the title [WIP] change edit courses frontend change edit courses frontend Dec 7, 2023
Copy link
Contributor

@rgu0114 rgu0114 left a comment

Choose a reason for hiding this comment

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

The code looks good to me but when testing on my end, the proportions on some screen sizes seem off. Could you double check that this fits the designs on a range of screen sizes?
Laptop:
Screen Shot 2024-03-14 at 4 00 47 PM
Mobile width messes with things 😕:
Screen Shot 2024-03-14 at 4 02 51 PM

Copy link
Contributor

@rgu0114 rgu0114 left a comment

Choose a reason for hiding this comment

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

Thanks for working so hard on these changes! Looking at the edits you made, I think it's looking a lot better. Could you update the most recent dependencies/fix merge conflicts though? Having trouble running locally.

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.

None yet

4 participants