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

GROW-130 Featured Loans #1953

Merged
merged 1 commit into from
Jul 31, 2020
Merged

GROW-130 Featured Loans #1953

merged 1 commit into from
Jul 31, 2020

Conversation

eddieferrer
Copy link
Contributor

  • Implemented features loans on the lend by category homepage

@@ -194,15 +194,15 @@ $card-half-space: rem-calc(14/2);
box-shadow: 0 0.65rem $card-margin $card-half-space rgb(153, 153, 153, 0.1);

&__image-wrapper {
height: rem-calc(168);
height: rem-calc(165);
Copy link
Contributor Author

@eddieferrer eddieferrer Jul 31, 2020

Choose a reason for hiding this comment

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

I made some very minor style tweaks here in this component.

.lend-by-category-homepage {
overflow: hidden;
}

.featured-loans {
padding: 2rem 0 1rem;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This section and .loan-categories dont have the same padding as per the design

* Implemented features loans on the lend by category homepage
@eddieferrer
Copy link
Contributor Author

This PR has the basic functionality of the featured loans carousel.

Now that we have preFetch working again on homepage subcomponents. We can prefetch the categories setting, then prefetch the category information, and the featured loan for the first category.

Issues I'm aware of and will followup on.

  • Not currently filtering funded loans based on fundedAmount/reservedAmount, I suppose if a funded loan comes back as the featured loan, we should fetch another loan. I wonder if this is worth doing, as it would be 2 sequential API requests, and that could take longer than the 5 seconds the slide takes to auto slide. Maybe we just show a funded loan? I think we should consider filtering funded loans on the backend and passing that as a param so that they are not returned.
  • Currently the featured loans carousel and the first loan on the category carousel will be the same. I will fix this in a followup PR.

@emuvente
Copy link
Collaborator

Not currently filtering funded loans based on fundedAmount/reservedAmount

This could also be handled by requesting 3-4 loans in the initial query and displaying the first one that isn't funded, which should work for most cases. For the rare case where all of the loans from the initial query are funded, a second query could be done at that point to get another 3-4 loans.

Handling it on the backend is an option too, but we would have to figure out what to do with the limit and offset parameters.

Copy link
Collaborator

@emuvente emuvente left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@ryan-ludwig ryan-ludwig left a comment

Choose a reason for hiding this comment

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

Code looks great 👍
Had some fussy invariant violations while trying to test but I trust it's just my VM.

Copy link
Contributor

@BoulderBrains BoulderBrains left a comment

Choose a reason for hiding this comment

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

👏

@eddieferrer eddieferrer merged commit 9ad12a3 into master Jul 31, 2020
@eddieferrer eddieferrer deleted the GROW-130_featured_loans branch July 31, 2020 19:37
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.

4 participants