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

Fixed UI Inconsistencies with Figma (LocationCards and ApartmentCard) #358

Merged
merged 15 commits into from
May 5, 2024

Conversation

kea-roy
Copy link
Member

@kea-roy kea-roy commented Mar 24, 2024

Summary

Background

After feedback from designers and devs, our team found many inconsistencies between the designs on Figma and the implemented design in the main branch. This pull request fixes inconsistencies found on the LocationCards and ApartmentCard components.

Changes

This pull request contains the following changes:

  • Fixed LocationCard and LocationCards styling to match the Figma design.
    • Adjusted the spacing
    • Adjusted the drop shadow
    • Added the hover effect
    • Adjusted the font styling and margin/padding values for responsiveness.
    • Adjusted the image styling to account for rounded SVGs, so the style matches the Figma on all screen sizes.
  • Fixed ApartmentCard styling to match the Figma design.
    • Adjusting the grid layout
    • Adjusting the spacing
    • Adjusting the font styling
    • Adjusting the responsiveness
    • Reworking the HeartRating component to support different numerical font sizes
    • Added hover effect for saved icon
  • Added documentation for LocationCard, LocationCards, and HeartRating components.

Test Plan

Compared design to Figma using Chrome's developer tools. I tested out the design on desktop-sized screens and mobile screens. Below are some images comparing the design in Figma to the coded design.

LocationCards Old Design

00 HomePage locationcards old design

Location Cards Figma

01 HomePage locationcards figma

Location Cards New Design

02 Homepage locationcards new code

ApartmentCard Old Design

00 HomePage ApartmentCard old design

ApartmentCard Figma

01 HomePage ApartmentCard figma

ApartmentCard New Design

02 Homepage ApartmentCard new code

ApartmentCard Mobile Old Design

00 HomePage ApartmentCard Mobile old design

ApartmentCard Mobile Figma

01 HomePage ApartmentCard Mobile figma

ApartmentCard Mobile New Design

02 Homepage ApartmentCard Mobile new code

…sign. (Spacing, Drop shadow, Hover effect, Responsiveness)
@dti-github-bot
Copy link
Member

dti-github-bot commented Mar 24, 2024

[diff-counting] Significant lines: 281.

Copy link
Contributor

@ggsawatyanon ggsawatyanon left a comment

Choose a reason for hiding this comment

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

You did a good job matching the UI inconsistencies with the Figma but I noticed that the height of the white part of the location cards is too long. Could you fix this? It is especially noticable on mobile version.
Figma:
image
On mobile:
image
On desktop:
image

frontend/src/components/Home/LocationCard.tsx Outdated Show resolved Hide resolved
@kea-roy
Copy link
Member Author

kea-roy commented Apr 9, 2024

You did a good job matching the UI inconsistencies with the Figma but I noticed that the height of the white part of the location cards is too long. Could you fix this? It is especially noticable on mobile version. Figma: image On mobile: image On desktop: image

Thanks Grace for pointing out the issue, I resolved the issue and updated the pull request!

@kea-roy kea-roy changed the title Fixed UI Inconsistencies with Figma Fixed UI Inconsistencies with Figma (LocationCards and ApartmentCard) Apr 9, 2024
@kea-roy kea-roy marked this pull request as ready for review April 9, 2024 21:01
@kea-roy
Copy link
Member Author

kea-roy commented Apr 10, 2024

Commit Changes

  1. Added hover effect for saved button on ApartmentCard

@kea-roy
Copy link
Member Author

kea-roy commented May 3, 2024

Commit Changes

  1. Simplified CSS code to resolve merge conflicts by removing unused CSS classes.

@kea-roy kea-roy dismissed ggsawatyanon’s stale review May 3, 2024 21:56

Completed the requested changes

…nimation for ribbon. improved readability of ternary operators for saveRibbon
…-dti/cu-apts into fixed-ui-inconsistencies"

This reverts commit 4f14646, reversing
changes made to 234e365.
… hover animation for ribbon. improved readability of ternary operators for saveRibbon"

This reverts commit 234e365.
@kea-roy
Copy link
Member Author

kea-roy commented May 4, 2024

Commit Changes

  1. Fixed Save Ribbon Styling
  2. Simplfied Save Ribbon ternary operators for readability
  3. Added save ribbon hover animation

Copy link
Contributor

@ggsawatyanon ggsawatyanon left a comment

Choose a reason for hiding this comment

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

Good job on fixing the inconsistencies and the ribbon icon. In the future, try to avoid reverting a merge commit because it can cause a lot of issues when trying to merge main into the branch. You can reset the merge commit before pushing to remote.

@ggsawatyanon ggsawatyanon merged commit b8bd30d into main May 5, 2024
5 checks passed
@ggsawatyanon ggsawatyanon deleted the fixed-ui-inconsistencies branch May 5, 2024 23:03
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