Skip to content

New modal component #934

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

olivierauverlot
Copy link
Contributor

New modal component and upgrade of the button component to allow the opening of a modal box.

('contents','A paragraph of text to display, without any formatting, without having to make additional queries.','TEXT',FALSE,TRUE),
('contents_md','Rich text in the markdown format. Among others, this allows you to write bold text using **bold**, italics using *italics*, and links using [text](https://example.com).','TEXT',FALSE,TRUE),
('unsafe_contents_md','Markdown format with html blocks. Use this only with trusted content. See the html-blocks section of the Commonmark spec for additional info.','TEXT',FALSE,TRUE),
('scrollable','Dreate a scrollable modal that allows scroll the modal body.','BOOLEAN',TRUE,TRUE),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
('scrollable','Dreate a scrollable modal that allows scroll the modal body.','BOOLEAN',TRUE,TRUE),
('scrollable','Create a scrollable modal that allows scroll the modal body.','BOOLEAN',TRUE,TRUE),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, corrected

Comment on lines 14 to 16
{{~#if unsafe_contents_md~}}
{{{markdown unsafe_contents_md 'allow_unsafe'}}}
{{else}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we remove this ? This is a more general problem with sqlpage and I think it should be either everywhere or nowhere.

What we could have instead is an embed property (same as in the card component) to embed another page. This is safe and more user-friendly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll try to implement embed, but I'll need to make the modal box fit the dimensions of the embedded content.

@lovasoa
Copy link
Collaborator

lovasoa commented Jun 8, 2025

Thank you very much for this contribution ! Are you using this component already in your projects ? In which context ? The inability to dynamically open the modal seems a little bit impractical.

@olivierauverlot
Copy link
Contributor Author

olivierauverlot commented Jun 9, 2025

Thank you very much for this contribution ! Are you using this component already in your projects ? In which context ? The inability to dynamically open the modal seems a little bit impractical.

Yes, I use it in my projects. The opening of modal boxes is triggered by the Button component. I use them to display additional information or contextual help. Modal boxes can also be opened with javascript.

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