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

Fix flash #29

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

Fix flash #29

wants to merge 2 commits into from

Conversation

bflatau
Copy link
Contributor

@bflatau bflatau commented Oct 7, 2022

I took the alert flash out of the whole app and applied to just the home page. I don't think it's needed to once a user logs in. I left it as-is for when a user first registers. I feel like the subsequent layout updates will help determine the best location for the register message.

This includes my edits for a previous hacktoberfest issue.

@nskins
Copy link
Owner

nskins commented Oct 7, 2022

Hey @bflatau! First of all, thanks for the PR! I think I'm looking for the alert to pop a little more. I want the user's attention directed to it whenever it appears. One idea I had was a toast notification of some sort. Maybe it appears at the top right of the screen for ~5 seconds? Also, we'll need to consider the alert-danger as well. So far, you've only updated alert-info.

In addition, we do need to consider the other screens as well (not just the home page). If you search for the function put_flash in the project, it shows up in various locations. Like if someone tries to access /items before logging in, it will create a danger alert on the login page (that's broken right now because my auth_form.html.heex doesn't show alerts yet).

This issue is a bit complex, so don't worry about it if it's more than you're bargaining for. I'd have to do quite a bit of research myself to figure out a good implementation...

@bflatau
Copy link
Contributor Author

bflatau commented Oct 11, 2022

Hey @nskins my pleasure! Yeah, I figured this was more complicated ha. I'll leave it alone for now. Let me know if you have any direction for the tailwind stuff or anything else I can contribute.

Thanks!

--Ben

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.

2 participants