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

edly-academy-ui-design-changes #9

Merged
merged 3 commits into from
May 30, 2024

Conversation

mubeenfayyaz20
Copy link
Contributor

@mubeenfayyaz20 mubeenfayyaz20 commented May 28, 2024

Home Page:

  • Container size is bigger than the proposed design (Done)
  • Filter dropdown padding is more than the proposed design (Done)
  • Small “a” in “All” filter (Done)
  • On clicking the second filter when the first one is already in the pressed state, the first one should be closed. Right now both stay open simultaneously. (Done)
  • Clicking outside should close the filter dropdown.
  • Resource cards are way bigger than the proposed design with extra margins and padding. (Done)
  • Icons used in labels are different from the ones provided in the design.
  • Padding in tags is inconsistent.
  • "What is Edly Academy?” section has too much padding between the heading and paragraph.
  • Footer is completely different from the proposed design.

Resource Page:

  • Videos should also be left aligned like the rest of the content.
  • There is a lot of padding between the heading and the paragraph. Need to decrease it.
  • Padding between two sections should be increased.

Other things:

  • Footer design was discussed with you so the rest is good I decreased the spaces on the footer.
  • Also, I compared all the changes in Figma and encountered many UI issues, but I managed to fix them as well. (Home and resource page)
  • The mobile responsiveness is good. Also, please set up the hamburger UI popup and elements.

@hinakhadim hinakhadim requested a review from regisb May 28, 2024 05:41
Copy link
Collaborator

@regisb regisb left a comment

Choose a reason for hiding this comment

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

Awesome! I only have minor comments. Can you please add before/after screenshots in your PR?

layout/base.html Outdated Show resolved Hide resolved
layout/index.html Outdated Show resolved Hide resolved
layout/static/js/search.js Outdated Show resolved Hide resolved
@mubeenfayyaz20
Copy link
Contributor Author

Feedback addressed 👍

Copy link
Collaborator

@regisb regisb left a comment

Choose a reason for hiding this comment

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

awesome, merging now :)

@regisb regisb merged commit 56a9e8d into edly-io:main May 30, 2024
1 check passed
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