Differentiating landlord reviews #305
Conversation
jesshan700
requested review from
AnkitLakkapragada,
cyrus14,
thuypham03,
mluo24 and
ggsawatyanon
|
Thuy Pham seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
[diff-counting] Significant lines: 64. |
|
Visit the preview URL for this PR (updated for commit e5eefa8): https://cu-apts-staging--pr305-link-apartment-landl-tw8ueh6w.web.app (expires Tue, 14 Nov 2023 18:14:53 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 096ac87b789b31770a01964fe0aaa92d563b9353 |
There was a problem hiding this comment.
Nice work Jessica! You really made the indicator look nice, and it's clear which one is a landlord review. Maybe you can also add indication for what's an apartment review with the apartment name?
Your logic with your solution is great, but I left some comments for how to micro-optimize it.
| {/* Checking to see if aptId of the review is null or empty, indicating that the review is a | ||
| landlord apartment - if landlord apartment is true, will add landlord review text */} | ||
| <Typography className={landlordIndicator}> | ||
| {review.aptId === '' || review.aptId == null |
There was a problem hiding this comment.
I think you can do !review.aptId instead.
Also, with what I was saying above, if you add landlordName as a prop, you can rewrite the code as such:
{landlordName && (
<Typography className={landlordIndicator}>
'Landlord Review - ' + landlordName
</Typography>
)}
This also only renders the component if landlordName has an input.
| //Retreieving landlord data | ||
| useEffect(() => { | ||
| get<Landlord>(`/api/landlord/${review.landlordId}`, { | ||
| callback: setLandlord, | ||
| }); | ||
| }, [review]); | ||
|
|
There was a problem hiding this comment.
I think this API call isn't actually necessary. The parent component already should have made this API call and acquired the data already (since it's the landlord page), and since you only need the name of the landlord, it would make sense to just pass the name of it as a prop to this component, especially since this component is used in other places where you don't need the landlord name.
I would suggest adding an optional prop to this component that passes in the landlord name, and in 188 replace the landlord?.name with this prop, and maybe change the condition so it relies on the landlord name being not null instead.
|
Updated the differentiation between landlord and apartments so that apartment links with the specific apartment shows on the card rather than showing the landlord review. Changed the styling so that it matches with the admin page. 
Redirects user to the specific apartment of that card. 
|
Changed styling of properties indicator rather than red link as shown above, better indicator that the link represents the property. Underlined portion is a link to the respective apartment page. |
…dti/cu-apts into link-apartment-landlord
…dti/cu-apts into link-apartment-landlord
Added to landlord reviews
This pull request is the first step towards showing the difference between landlord reviews and apartment reviews on the landlord page. Checks the aptId field for each review to see if it is empty or null indicating a landlord review.
Test Plan
Testing Lambrou landlord vs apartment:
Testing Travis Hyde landlord vs apartment:
Notes
Breaking Changes