-
-
Notifications
You must be signed in to change notification settings - Fork 528
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
Add Panel Modal #5896
Add Panel Modal #5896
Conversation
panel/layout/modal.py
Outdated
|
||
|
||
_template = """ | ||
<style id="pnx_dialog_style"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should maybe find another way to include the style. The problem is that with the fast templates this style needs to be available globally while with other templates it does not have to.
Whether or not the modal is open. Set to True to open. Set to False to close.""") | ||
show_close_button = param.Boolean(True, doc="Whether to show a close button in the modal") | ||
|
||
open = param.Event(doc="Click here to open the modal") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- I would like to have actions instead of events as they are more natural to use from Python. But when I do, I get serialization error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you expand on this? We explicitly introduced Event because Action
seemed weird and hacky.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because its more natural than an action from the Python side as a function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guess that's true, the problem was that param.Action(lambda p: p.param.trigger('open'))
just is a very bizarre thing. I do agree from an enduser perspective it's clearer than having to call .param.trigger('open')
yourself though.
} | ||
""" | ||
|
||
class Modal(ReactiveHTML, NamedListLike): # pylint: disable=too-many-ancestors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Should figure out if its possible to add parameter to disable the closing of the modal when clicking the back drop.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5896 +/- ##
==========================================
- Coverage 84.41% 84.19% -0.23%
==========================================
Files 291 293 +2
Lines 42655 42811 +156
==========================================
+ Hits 36007 36043 +36
- Misses 6648 6768 +120
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
This PR migrates the Panel-Modal to Panel as agreed with Philipp. Having a standalone modal provides more flexibility than the modal built into templates. For example it also works in notebooks. The user experience is also better.