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

Dissmisable alerts #3154

Closed
wants to merge 0 commits into from
Closed

Conversation

fettuccinae
Copy link
Contributor

Problem

The alert wasnt dismissable and it felt like it reflected the status of user's last.fm connection.
https://tickets.metabrainz.org/browse/LB-1728

Solution

Added a button which onclick disables the visibility of alert.

Copy link
Member

@MonkeyDo MonkeyDo left a comment

Choose a reason for hiding this comment

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

I think that is a good idea, thanks for opening a PR !

There is a simpler way to dismiss alerts. You don't need a stats variable, just the close button with no onClick param, and a few relevant css classes: you can follow the steps in
https://getbootstrap.com/docs/4.0/components/alerts/#dismissing

The button html content is very similar to what you currently have:

<button type="button" class="close" data-dismiss="alert" aria-label="Close">
   <span aria-hidden="true">&times;</span>
 </button>

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