Skip to content

[Demo] Infinite Scroll 2/2 #1887

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

Merged
merged 1 commit into from
Jun 6, 2024
Merged

Conversation

smnandre
Copy link
Member

@smnandre smnandre commented Jun 4, 2024

Q A
Bug fix? no
New feature? no
Issues Fix
License MIT

This took some time but.... what a beautiful collection of TShirts 😸

Capture d’écran 2024-06-04 à 20 40 14

@carsonbot carsonbot added demo Demo on ux.symfony.com Status: Needs Review Needs to be reviewed labels Jun 4, 2024
@smnandre smnandre requested review from kbond and WebMamba June 4, 2024 19:02
@seb-jean
Copy link
Contributor

seb-jean commented Jun 5, 2024

I just tested it, and it works perfectly! 👏

Copy link
Contributor

@WebMamba WebMamba left a comment

Choose a reason for hiding this comment

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

Thanks @smnandre, just tested locally it's working great!

<p>
Let's focus on two aspects. But first, <code>did you scroll</code> all the way to the end?
<br>
You might have missed some important details about performance... 🍿
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could explain the performance details you mention, I don't get what you are thinking about 🙃

Copy link
Member Author

Choose a reason for hiding this comment

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

The "750ms delay" ... there is a pragraph at the end of the grid

Comment on lines +64 to +66
🐯 For the previous page results, we add one fake item
corresponding to the last item of the previous page,
with the same `id`. This allows the **addition of new elements** after the existing ones.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could go a little deeper here and explain what morphism is and what morphism does with this same id?
And I think this part should be after the observer part?

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe we could go a little deeper here and explain what morphism is and what morphism does with this same id?

I think this would require a dedicated page (and i have the perfect slide for this remember ? 😆 )

Copy link
Member Author

Choose a reason for hiding this comment

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

And I think this part should be after the observer part?

Oh i remember now... we need to define why the div 'previous' / 'current' / 'next' at first, because elsewhen this is impossible to understand ... :/

To be honest i'm not entirely ok with the final result, and i really want to create something more interactive / steps by steps highlither, etc..

I'd say "this is ok" for this one, but i'm really trying a lot of things these days to "more"

Comment on lines +61 to +72
{% if not this.hasMore %}
<div style="display: grid; place-content: center;padding-block: 2rem;">
<div style="max-width: 36rem;">
<h4>Did you know?</h4>
<p>
This demo adds a fixed delay of <code>750ms</code> before each page load to simulate slow
network conditions. Or maybe just to let you enjoy the sweet animations... 🍿
</p>
<p>Wonder how it would feel without? Why not get the code and try it yourself?</p>
</div>
</div>
{% endif %}
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the performance thing @WebMamba :)

@kbond kbond force-pushed the demo/infinite-scroll-2 branch from ed52382 to a23123c Compare June 6, 2024 23:09
@kbond
Copy link
Member

kbond commented Jun 6, 2024

Looks great, thanks Simon!

@kbond kbond merged commit b797732 into symfony:2.x Jun 6, 2024
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
demo Demo on ux.symfony.com Status: Needs Review Needs to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants