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

feat: add to basket experiment mp-346 #5380

Merged
merged 15 commits into from
Jul 16, 2024
Merged

Conversation

christian14b
Copy link
Collaborator

@christian14b christian14b commented Jul 11, 2024

Bubble up an small borrower image to the new basket and show cart modal

add-to-basket-exp.mov

@christian14b christian14b marked this pull request as ready for review July 12, 2024 22:20
@christian14b christian14b requested review from emuvente and a team July 15, 2024 16:03
@eddieferrer
Copy link
Contributor

Do we need to account for Anonymous loans in this work? I can't remember if Anonymous loans return "Anonymous" in the name field. I know they are few and far between, I can't even find one at the moment. @emuvente

@emuvente
Copy link
Collaborator

Do we need to account for Anonymous loans in this work?

Loans can only become anonymous after the fundraising period, so it shouldn't be a problem for this work. When loans are anonymous then the name does only come through from the API as "Anonymous".

@dyersituations
Copy link
Collaborator

I'm now seeing the page content always under the header in /lend/filter:
image

@dyersituations
Copy link
Collaborator

So it looks like we're just enabling this on the category pages? Or do we also want it enabled on /lend/filter? If we want it on /lend/filter, create a mixin like we have for other experiments, and enable the experiment on the filter page. Otherwise, if we only want the experiment on the category page, move the experiment tracking back to the channel component. Sorry if that's the case, I assumed we were rolling this checkout experience to as many pages as possible.

@dyersituations
Copy link
Collaborator

While testing on the category page, I also noticed that sometimes the animated bubbles go behind the other loan cards on the page. Make sure the z indexing is correct to make the bubbles always be on the top. I mostly noticed when I clicked a button on the left side of a row.

@dyersituations
Copy link
Collaborator

Is it expected that the tablet view is top left of the screen (not full-width or right-side)?
image

@dyersituations
Copy link
Collaborator

I noticed that the header doesn't become sticky until you add a loan to the basket. This means that there is a weird jump in the page layout when you are scrolled down a bit and add a loan to the basket. The jump can be seen more in mobile view. Would we rather want to have the header be sticky always for the experiment (whether there are loans in the basket or not)?

@christian14b
Copy link
Collaborator Author

Is it expected that the tablet view is top left of the screen (not full-width or right-side)? image

Thanks for checking, this should be fixed with kiva/kv-ui-elements#428

@christian14b
Copy link
Collaborator Author

I noticed that the header doesn't become sticky until you add a loan to the basket. This means that there is a weird jump in the page layout when you are scrolled down a bit and add a loan to the basket. The jump can be seen more in mobile view. Would we rather want to have the header be sticky always for the experiment (whether there are loans in the basket or not)?

Yeah.. your solution seems like the most straightforward. I tried to make it sticky after the viewport but it looks weird because the cart modal appears with margin top value of the header's height on desktop. @emuvente what do you think?

@emuvente
Copy link
Collaborator

I noticed that the header doesn't become sticky until you add a loan to the basket. This means that there is a weird jump in the page layout when you are scrolled down a bit and add a loan to the basket. The jump can be seen more in mobile view. Would we rather want to have the header be sticky always for the experiment (whether there are loans in the basket or not)?

Yeah.. your solution seems like the most straightforward. I tried to make it sticky after the viewport but it looks weird because the cart modal appears with margin top value of the header's height on desktop. @emuvente what do you think?

I don't think we want to test making the header sticky always as part of this experiment. Could we try using a transition to slide the header in from the top instead of a sudden jump?

@christian14b
Copy link
Collaborator Author

I noticed that the header doesn't become sticky until you add a loan to the basket. This means that there is a weird jump in the page layout when you are scrolled down a bit and add a loan to the basket. The jump can be seen more in mobile view. Would we rather want to have the header be sticky always for the experiment (whether there are loans in the basket or not)?

Yeah.. your solution seems like the most straightforward. I tried to make it sticky after the viewport but it looks weird because the cart modal appears with margin top value of the header's height on desktop. @emuvente what do you think?

I don't think we want to test making the header sticky always as part of this experiment. Could we try using a transition to slide the header in from the top instead of a sudden jump?

Yeah, I added transition in my last commit just to soft page jump

@christian14b christian14b merged commit 0cd52cf into main Jul 16, 2024
5 checks passed
@christian14b christian14b deleted the add-to-basket--MP-346 branch July 16, 2024 21:24
@kiva-robot
Copy link
Collaborator

🎉 This PR is included in version 2.748.0-rc.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@kiva-robot
Copy link
Collaborator

🎉 This PR is included in version 2.748.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants