Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 a standalone Modal layout #7083
Add a standalone Modal layout #7083
Changes from all commits
5d211f4
d301572
7dcf020
4220c27
6d1689f
009c57d
6cea115
e8585a9
185d5e2
5a82aca
e3c8e75
1484cc3
e68d42f
7145d8b
8a617aa
256b337
df0140a
d5f8501
09c414d
7f9b12b
94a27fc
fea45f0
c9ada92
06aee3f
cf3414e
1bb3c55
e784495
52454dc
1e1ded7
5e82f98
59bba8a
5914e4e
9d531b1
77e5adc
8235a48
230e81e
1f51a84
6448b40
77a2ff7
346afa7
c460798
925c57d
d10ea1a
c573c0e
2d17f1b
7b8017d
ea9dbdd
8ca8a61
06a0de2
504ad4c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
One thing to consider is if the
Modal
could be made easier to use? Could the api of theModal
be generalized to a concept of actions/ action buttons.Could the api be simplified?
The api of
panel-modal
that I made is:It requires you define a variable holding the Modal and include both the button (
modal.param.open
) and the modalmodal
.If you want to customize the button it requires something like
pn.widgets.Button.from_param(modal.param.open
as well.Could the api be simplified? Could it be enough to just include the modal?
That should show the button. And when clicked the modal should open/ close. That would make it much simpler to drop in as you don't need to define a variable holding the
Modal
.If you want to customize the button you can just do:
If you want to trigger the modal from something else than a Button than you should be able to hide the button:
Could the API be generalized?
A Modal is actually a part of a general concept for action buttons. You want to be able to trigger an action via a Button. You want to be able to easily set the source or target (
object
) and be able to customize the button.I've had in the back of my head that I wanted to create a
panel-action-buttons
package one day. Compiling these as I use those for my work app all the time.@philippjfr. What do you think?
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.
Sorry I never responded, especially as we're working towards merging this. My feeling is that while I can see how it might be useful it kind of breaks a few mental models people have built. A few more lines of code seem like an okay price to pay for keeping a relatively consistent mental model for how things worked. For a
Modal
to render as a button and something likepn.widgets.Button.from_param(Modal())
to work we'd also internally have to break a lot of models on how things work, so I'm roughly -1 on this, though it's something we could think about adding later.The one thing we could consider doing to make this potential option backward compatible would be to only allow the modal to be rendered as a root, i.e.
pn.Row(modal).servable()
would be disallowed andmodal.servable()
would be the only valid way to add a modal. What do you think @hoxbro?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 added
Modal.create_button
after this comment to make it easier to use the Modal.I'm not sure about the suggested API. I'm not hard against it. If you still want this, maybe open a new issue to discuss it.