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

Alert pane: Allows providing contextual feedback messages for typical user actions #1181

Merged
merged 5 commits into from
Jun 17, 2020

Conversation

MarcSkovMadsen
Copy link
Collaborator

@MarcSkovMadsen MarcSkovMadsen commented Mar 23, 2020

One of the things i use a lot and see other users using in their analytics app are Bootstrap inspired Alerts.

So I would like to contribute one.

The contributed code is ready for review by @philippjfr .

I've chosen

  • To implement one Alert with an alert_type parameter instead of a collection of PrimaryAlert, ..InfoAlert, ..., DarkAlert to keep the api in line with Button.
  • To add an alert.css file instead of styling inline. That will enable easier custom styling.
  • Not to implement a custom Bokeh model supporting the alert_type. Let me know if that is needed.
  • To include it in pn.pane instead of pn.widget or pn.components.bootstrap. Those would be alternatives.
  • To make the 'stretch_width` by default. I believe this is normal behaviour.
  • To not implement the alert-link css class. Instead any <a> link tag will just be styled.
  • To not implement the alert-dismissible js functionality. Would be nice to have though.

Alert notebook reference example

image

image

image

Server test example

python -m panel serve 'panel\tests\pane\test_alert.py' --dev --show

alert

Additional Context

This is an example of where the css_classes parameter does not work that well in Panel @philippjfr . In order to get the proper styling you need to define .bk.alert-primary instead of .alert-primary to get specific enough. That means you can't copy-paste/ or include one of the many existing style sheets out there. This is friction. :-)

@MarcSkovMadsen MarcSkovMadsen added the type: feature A major new feature label Mar 23, 2020
@philippjfr
Copy link
Member

I've heard some hopeful things about eventually being able to remove the css prefixing but I doubt that'll happen in the near future.

@philippjfr
Copy link
Member

Thanks for the contribution though, very sensible addition.

@MarcSkovMadsen
Copy link
Collaborator Author

A small step for man :-)

panel/pane/alert.py Outdated Show resolved Hide resolved
@philippjfr
Copy link
Member

Apart from my minor two comments this is mergeable. Could you maybe also generate a thumbnail for the gallery and upload it here? That way I can add it to the CDN and it'll find it during the next doc build.

@MarcSkovMadsen
Copy link
Collaborator Author

MarcSkovMadsen commented Mar 24, 2020

One question @philippjfr is whether the type and colors of button and Alert should somehow be aligned?

image

image

And maybe even aligned with the Card I'm contributing. Maybe the card should have a card_type and associated header colors?

image

@philippjfr
Copy link
Member

One question @philippjfr is whether the type and colors of button and Alert should somehow be aligned?

Might be nice, maybe we should have a saturated=True/False option to toggle between the pastel and fully saturated colors?

@MarcSkovMadsen
Copy link
Collaborator Author

One question @philippjfr is whether the type and colors of button and Alert should somehow be aligned?

Might be nice, maybe we should have a saturated=True/False option to toggle between the pastel and fully saturated colors?

...hmmmm.... I'm not an expert on colors. Try to stay out of it but always gets to spend a lot of time on it. Any experts around?

panel/pane/alert.py Outdated Show resolved Hide resolved
@philippjfr philippjfr added this to the v0.10.0 milestone Apr 2, 2020
@philippjfr
Copy link
Member

Merging for now, can add the saturated colors later.

@philippjfr philippjfr merged commit 626ac27 into holoviz:master Jun 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature A major new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants