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

Auto close certain toast messages based on setting #236

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

elfjes
Copy link
Contributor

@elfjes elfjes commented Nov 13, 2024

No description provided.

Copy link
Collaborator

@hmpf hmpf left a comment

Choose a reason for hiding this comment

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

LGTM but @podliashanyk should have a say.

Copy link
Contributor

@podliashanyk podliashanyk left a comment

Choose a reason for hiding this comment

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

I haven't run the code yet to check the rendered HTML. But I suggest that we drop the approach with opacity completely. It will make the toast transparent but it is still present and will take up space in the DOM (just check with 2 alerts, one that is "removed" and 1 that is not). But what is worse is that since it is now transparent the user can't see it to close it. So it is potentially permanently there.
I still think that using the remove me HTMX extension approach is perfect for this use case.

@elfjes
Copy link
Contributor Author

elfjes commented Nov 14, 2024

I haven't run the code yet to check the rendered HTML. But I suggest that we drop the approach with opacity completely. It will make the toast transparent but it is still present and will take up space in the DOM (just check with 2 alerts, one that is "removed" and 1 that is not). But what is worse is that since it is now transparent the user can't see it to close it. So it is potentially permanently there. I still think that using the remove me HTMX extension approach is perfect for this use case.

the hyperscript reads:

on load wait {{ autoclose }}s then add .autoclosing to me then wait 0.3s then remove me

so it will first add the autoclosing (which transitions in 0.25s), and then will remove the element completely :)

the opacity change is just for making a nice transition

@podliashanyk
Copy link
Contributor

I haven't run the code yet to check the rendered HTML. But I suggest that we drop the approach with opacity completely. It will make the toast transparent but it is still present and will take up space in the DOM (just check with 2 alerts, one that is "removed" and 1 that is not). But what is worse is that since it is now transparent the user can't see it to close it. So it is potentially permanently there. I still think that using the remove me HTMX extension approach is perfect for this use case.

the hyperscript reads:

on load wait {{ autoclose }}s then add .autoclosing to me then wait 0.3s then remove me

so it will first add the autoclosing (which transitions in 0.25s), and then will remove the element completely :)

the opacity change is just for making a nice transition

Right... didn't notice the hyperscript 🙃 OK, that promises the remove-me extension functionality then

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Changes requested
Development

Successfully merging this pull request may close these issues.

3 participants