digitalbazaar / bedrock-angular-authn

Other
0 stars 3 forks source link

Replace bootstrap `alert` classes #4

Open mattcollier opened 7 years ago

mattcollier commented 7 years ago

https://github.com/digitalbazaar/bedrock-angular-authn/blob/master/login-modal-component.html#L7-L10

We're using bootstrap alert classes here.

Material does not appear to have equivalent classes we can use. Do we want to use custom CSS and if so, where should we put it?

This SO post suggest lifting the alert classes out of bootstrap: https://stackoverflow.com/a/40059571

dlongley commented 7 years ago

Yeah, I noticed their absence but never got around to solving this issue before.

This is another approach:

https://mdbootstrap.com/angular/advanced/alerts/

Angular-material has "toast": https://material.angularjs.org/latest/demo/toast

Perhaps we can colorize it and do alerts that way, as overlays.

mattcollier commented 7 years ago

The alert classes are used in the demo warning as well: https://github.com/digitalbazaar/bedrock-angular-demo-warning/blob/master/demo-warning-component.html#L1

dlongley commented 7 years ago

I think we should explore creating a simple br-alert directive that uses $mdToast and angular material's theme provider stuff. A br-alert would have a br-show attribute for when to show/hide -- and, if we want, an option to add a close button to them that would auto-hide them.

Then we can replace wherever we use bootstrap alert classes with br-alert.

mattcollier commented 7 years ago

@dlongley sounds like a good thing to experiment with. Why br-show instead of using ng-if, ng-show etc?

dlongley commented 7 years ago

@mattcollier -- because that's how $mdToast works ... you have to call functions to show/hide it, from what I can tell. Probably has to do with animation and positioning.

dlongley commented 7 years ago

We should also update brAlerts to use this new br-alert as well. This should cause alerts to be uniform whether occurring in a form or via some other method.

mattcollier commented 7 years ago

@dlongley turns out, that we can use md-card to achieve the same effect we had before.

Here's the implementation in bedrock-angular-demo-warning

md-card-demo-warning

dlongley commented 7 years ago

@mattcollier,

Yeah, I did that before but I'm not convinced it's what we actually want. Maybe we have two different classes of alerts. I am thinking that md-toast will work much better for our brAlerts/brAlertService stuff. It could be that md-card will work ok for static, on screen alerts like our demo warning.

mattcollier commented 7 years ago

@dlongley we are in agreement, already implemented this for the static use case: https://github.com/digitalbazaar/bedrock-angular-alert/blob/material/alert-component.js

dlongley commented 7 years ago

If we're using br-alert for the static stuff, what component will we use for the "toast" stuff? Seems like "alert" is a better name for the toast-like stuff and we need another name for whatever these static things are. Thoughts?

mattcollier commented 7 years ago

@dlongley I thought br-toast would be an appropriate name for toast things.