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

Dialog component: multiple feature requests #4288

Closed
aedu-hostettler opened this issue Dec 15, 2024 · 3 comments
Closed

Dialog component: multiple feature requests #4288

aedu-hostettler opened this issue Dec 15, 2024 · 3 comments
Labels
✨ feature needs: input More input from the author of the ticket is needed to proceed.

Comments

@aedu-hostettler
Copy link

aedu-hostettler commented Dec 15, 2024

We are currently implementing the dialog component in our multiple webservices. (replacing bootstrap modals, and get rid of bootstrap dependencies entirely, so we will be ready for the design-system v9 update) could you possibly prioritise these feature requests accordingly? (we would need it "now" 😄)

Is your feature request related to a problem? Please describe it.

  • Limiting condition from our project: We can't use forms element within a dialog (we use one globally surrounding whole page content (without post-header/footer))
  • opening a dialog is not documented on storybook
  • closing a dialog independent of form submit event is not possible.
  • closing x button "btn-close" does nothing (same problem as in toasts/alerts (what is the intended difference between toasts and alerts? or is one deprecated?)

Describe the solution you'd like

  1. Dialog component should work without dependence on a containing forms element.
  2. Document on storybook that the dialog has a .showModal() function, that opens/shows the dialog.
  3. rename the .showModal() function to openDialog (now that it is not a bootstrap modal anymore.)
  4. add a .closeDialog (or hideDialog?) function, so the dialog is dismissible by other means, than the form submitted event. (either close/open or show/hide)
  5. link the closeDialog function (maybe by attribute) to the x-closing-button (situated on top right of dialog) by default, so no additional code is needed. (or by additional custom attribute described with point 6.)
  6. maybe add a custom attribute for the x-close button, but also that one can add the new custom attribute on a normal "cancel" button, that does close the dialog by default, without submiting a form.
  7. maybe also add a custom attribute for the initial opening button (outside of dialog component), that shows the dialog. so the initial opening button would only need a custom-attribute, and the target id of the dialog to open. (not relying on, that the dialog is the next sibling to the opening button)
  8. also add an optional custom attribute to add on the dialog element itself, so it would be shown initially on page-load without the need of custom js or button-click-action. (sometimes we want a dialog to be shown on page-load, blocking all other actions on the page, until an option on the dialog is selected)
  9. in the code example below i added some data-attributes as example but im not sure if the "data-" part is needed. maybe add attributes without "data-".

Describe alternatives you've considered
We would have to implement custom forms inside dialogs, that would be a huge hassle to deal with on our project scope. (nested forms not really working)

Additional context
Here would be some example code with comments, that gives more detail-infos:

`

                    @*instead of onclick js function, it would be nice, to have a custom attribute to set, with the htmlId of the dialog to open*@
                    <button type="button" id="show-dialog-button" class="btn btn-secondary" onclick="openDialog('#dialogExample')" or prefered data-dialog-open-id="dialogExample">
                        Show dialog by js or attribute
                    </button>

                    @*could you add another extra attribute, so the dialog would be shown on page load initially, without an action or button has to be pressed, or custom js has to be executed?*@
                    @*in some cases we load or show a page, and depending on serverside code, a modal should be shown and an action has to be selected, before interacting with the rest of the page.*@
                    <dialog id="dialogExample" class="bg-white" data-size="large" data-visibility-status="open/closed or shown/hidden">
                        @*we don't use a form within the dialogs, because we always have a globel forms surrounding the whole web content of our pages. so we use a div instead of a form element. is the "dialog-grid" css class needed, or could the dialog-grid wrapper element be omited? *@
                        <div class="dialog-grid">
                            <post-icon name="1034"></post-icon>
                            <h3 class="dialog-header">title</h3>
                            <div class="dialog-body">contenttext or form-elements of an outer/global forms element outside the dialog-component</div>
                            <div class="dialog-controls">
                                <button type="button" class="btn btn-primary">OK</button>
                                @*example of how we would use a OK or "submit" button, without actually submitting, but calling another/or source/same page, where modal would not be shown again.*@
                                <a class="btn btn-primary" href="javascript:gg.refreshView('someUrl') or postBack('/vgkklp/annahme/_avz_/avz/TeillieferungLoeschen/', null, 'bestaetigungTeillieferungLoeschen');" id="bestaetigungTeillieferungLoeschen" role="button" type="button"><span>Ja</span></a>
                                @*currently without custom js, the form would not get closed. could you maybe add a custom attribute, that closes the dialog on click/keyboard-enter?*@
                                <button type="button" class="btn btn-secondary" onclick="closeDialog('dialogExample')" or prefered data-dialog-close-id="dialogExample">Cancel</button>
                            </div>
                            @* without custom js, clicking or keyboard-enter does not close the dialog. could you maybe merge functionality with the btn-close css class or the same custom attribute that would close the dialog on a default cancel/no button? *@
                            <button type="button" class="btn btn-close" onclick="closeDialog('dialogExample')" or prefered data-dialog-close-id="dialogExample">
                                <span class="visually-hidden" >Schliessen</span>
                            </button>
                        </div>
                    </dialog>
                    <script>
                        function openDialog(elementId) {
                            if (elementId.startsWith('#')) {
                                elementId = elementId.replace('#', '');
                            }
                            //that is how we actually show a dialog currently. it is not documented in the dialog component page, (had to check sourcecode in examples) could you add more documentation?
                            //and now that it is not a bootstrap-modal, could you maybe rename it to "showDialog()"?
                            document.getElementById(elementId).showModal();
                        }

                        function closeDialog(elementId) {
                            if (elementId.startsWith('#')) {
                                elementId = elementId.replace('#', '');
                            }
                            //could you add a closeDialog or hideDialog function to the dialog component/tag, so it is possible to close it without relying on the form submit event? (but then if you would add a custom attribute for closing functionality, we wouldn't use the js function alternative in normal circumstances. But it would be nice to have the possibility to close it also by custom js, outside of the component scope.
                            document.getElementById(elementId).closeDialog();
                        }
                    </script>

`

@gfellerph
Copy link
Member

gfellerph commented Dec 16, 2024

One major thing to note here, is that the <dialog> element is a native HTML element and we're just styling it with CSS. All native functionality is available as described on the MDN docs for dialog. This means we're not able to change the APIs for opening/closing, they are defined and implemented by the browser.

If you're interfacing with the dialog via JavaScript, there is no need for the forms element. It's just handy to have if you don't even want to write JS to make the dialog work. Usually, we don't document native APIs, but in the case of the dialog it might make sense to link to the MDN docs at least and point out that this is a native element.

Concerning the nested forms, could you share some more details on your use-case, maybe some links to inspect? I'm not sure if I understand the context correctly.

With that said, I'd also like to answer your questions:

  1. Dialogs don't require form elements, it's just a neat option to have a dialog that does not need JS to work
  2. We'll improve the docs on this front
  3. Not possible since it's native API
  4. Already available, use the .close() method
  5. This would need additional investigation, but there might be an easier solution to this in the future. Until then, implementing something like const handleClick = (e) => e.target.closest('dialog').close() should be trivial. The recommended way though is using the standard approach with the <form method="dialog"> because it's future proof
  6. This is already available natively, so we'll not prioritise implementing something custom at this moment. It's always recommended to align with web standards
  7. This would be doable with a web component, but not a priority for us at the moment because it can easily be implemented
  8. The dialog element supports the <dialog open> attribute, but please note that this will show the dialog in a non-modal way which allows interaction with the page in the background (opening in modal mode by default is not supported yet, but discussed in standard bodies).
  9. It's recommended to always specify custom attributes with a dash as new native attributes will never use a dash (e.g. popovertarget = native vs. popover-target = custom). The only exception are aria-* attributes which already use a dash

Code:

  • Using onclick is not really recommended (I think we used it in the docs just out of laziness, will improve). Use Angular or JS listeners (https://stackoverflow.com/questions/5871640/why-is-using-onclick-in-html-a-bad-practice)
  • If you want to show the modal on page load, execute a small script on load like (function () { theDialog.showModal() })();
  • The .dialog-grid class helps you to layout the dialog according to the CI/CD, but if your content is different, there's no point in using this class
  • Why are you using a link tag if you redeclare it as a button with role="button" instead of just using a button?
  • Since for this component we're just using HTML/CSS, it's not possible to add functionality on top of what's available natively, adding custom functionality for things that are already possible to implement is not a priority at the moment, but we might think about adding it in the future

@gfellerph gfellerph added the needs: input More input from the author of the ticket is needed to proceed. label Dec 16, 2024
@aedu-hostettler
Copy link
Author

aedu-hostettler commented Dec 16, 2024

Hey @gfellerph .
Thank you so much for your answers.
I just wasn't aware it was an mdn or web standards dialog, and that it is well documented there.
I assumed it was a custom web-component of yours.

In that case, sorry for alot of useless feature-requests. 😄 .

Though improving the docs (your answer 2.) linking to the mdn doc of course would be appreciated.

If we don't have to use the form-element within a dialog, then there is no problem. No nested forms then.

ps: to answer your question:
Why are you using a link tag if you redeclare it as a button with role="button" instead of just using a button?

It is simply a leftover from a time, where no one was aware, that the button, without the type="button" attribute would submit the form, thus a-tags with type button were introduced at a lot of places. surely a thing to clean up (now). Our code-base is close to getting a 20 years celebration party 🥳. (not really getting a party, but it's not too easy to introduce angular to our "eco-system".)

Thx again for your answers, and sorry for the mix-up. I just wasn't aware that the dialog is a mdn/html standard.

I could close the ticket from my side, because everything is clear to me now, but i let you close it, in case you want to keep it open for the reminder of updating the dialog-docs with the mdn link.

@gfellerph
Copy link
Member

Closing as answered.

@github-project-automation github-project-automation bot moved this from 👀 Triage to 🚀 Done in Design System Production Board Mar 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ feature needs: input More input from the author of the ticket is needed to proceed.
Projects
Status: 🚀 Done
Development

No branches or pull requests

2 participants