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] Declarative indicator for dialog modality #920

Open
lukewarlow opened this issue Oct 31, 2023 · 9 comments
Open

[dialog] Declarative indicator for dialog modality #920

lukewarlow opened this issue Oct 31, 2023 · 9 comments

Comments

@lukewarlow
Copy link
Collaborator

The invokers proposal is a brilliant addition and really helps solve some of the developer ergonomic issues with dialogs (along with much more).

However, it misses one key case as I mentioned in whatwg/html#3567, you can't have a modal dialog load open with just HTML, you need JS to call showModal() or a user to click an invoker button.

My use case is that I sometimes have modal dialogs that are linked to a client side router where the URL controls their visibility. I currently call .showModal() on component mount. But if I could instead just do this declaratively that would remove the requirement for client JS execution (think SSR scenarios)

<dialog open> works for non-modals but there's no equivalent for modal dialogs.

@lukewarlow
Copy link
Collaborator Author

lukewarlow commented Oct 31, 2023

Happy to bikeshed etc but my proposal would be a new modal attribute. It wouldn't change anything by default it just acts as an indicator when used alongside the open attribute.

My thoughts on what <dialog modal> would do when called with JS.

show() - Throw an exception (I'm not fully set on this so could probably be persuaded either way).

showModal() - Work as expected.

<dialog popover modal> would be an author error and would throw in some cases. Not sure which one should win though.

This could also simplify the invoker proposal. Currently we have a toggleModal action, and Keith has added an additional toggle action inside of his Chromium CL.

If we added a new modal attribute, we could just have a single toggle action that would just work as expected?

Depending on the behaviour of show() JS function, we could even just have a show action and forget showModal?

@lukewarlow lukewarlow added agenda+ Use this label if you'd like the topic to be added to the meeting agenda invokers labels Oct 31, 2023
@lukewarlow
Copy link
Collaborator Author

lukewarlow commented Oct 31, 2023

Adding to agenda as it might help simplify the invokers work that's ongoing. (Would obviously need WHATWG buy in too)

@YummyBacon5
Copy link
Contributor

If we added a new modal attribute

Could <dialog open="modal"> not work?

@lukewarlow
Copy link
Collaborator Author

Could not work?

It would potentially for the open as modal on page load. Potentially as I'm not sure on changing boolean attribute to take values. I guess it was done with hidden so could be fine?

It wouldn't, however, additionally remove ambiguity around invoker actions though.

@lukewarlow lukewarlow changed the title [dialog] Way to set a dialog to modal and open on page load [dialog] Declarative indicator for dialog modality Oct 31, 2023
@keithamus
Copy link
Collaborator

keithamus commented Oct 31, 2023

Potentially as I'm not sure on changing boolean attribute to take values. I guess it was done with hidden so could be fine?

Yes this is fine as it will become an enumerated attribute where the invalid/empty states map to <whatever the current behaviour is>. The problem is that the current behaviour will need a name.

@lukewarlow
Copy link
Collaborator Author

Naming things is hard, could that be another reason to use a new attribute for the modality instead. Then we get potential invoker benefits and we don't have to think of naming

@mfreed7
Copy link
Collaborator

mfreed7 commented Nov 9, 2023

Some quick references for naming and behavior, since a declarative "open when loaded" feature was initially part of Popover but was removed.

@css-meeting-bot
Copy link

The Open UI Community Group just discussed [dialog] Declarative indicator for dialog modality.

The full IRC log of that discussion <jarhar> Luke: this is twofold. its related to invokers, but also a separate thing that it poentially important
<masonf> q+
<jarhar> Luke: invokers solves problems about dialog showmodal without js
<jarhar> Luke: doesnt solve problem that its opened by default on page load
<jarhar> Luke: generally not something you need to do, but using frameworks is something that they might want to do
<jarhar> Luke: for example, my edit root is a modal dialog that shows on top and thats rooted with a separate url
<jarhar> Luke: if i want to do that with ssr, you still need to run js on the frontend to showmodal
<brecht_dr> q+
<jarhar> Luke: discussed in whatwg issue, i think that the twofold benefit of this is that we have the modal attribute and the dialog is modal when opened
<jarhar> Luke: could simplify showmodal and togglemodal
<jarhar> Luke: if we made the modality is decided by the dialog rather than the action that could be simple
<jarhar> Luke: what are peoples thoughts?
<jarhar> Luke: modal attribute wouldnt open the dialog - separate open attribute, modal would just indicate that this will become a modal dialog
<jarhar> masonf: there was a feature for this for popover called defaultopen
<jarhar> masonf: there are some tricky bits. naming, there was a lot of conversation about naming. attribute name needs to communicate that it is currently modal
<jarhar> masonf: even having the open attribute on the dialog is a mistake
<jarhar> q+
<jarhar> masonf: put it back in the page, then its modal but not open anymore, and attributes cant be added while inserting or removing from the page
<jarhar> masonf: tricky with state changes
<jarhar> masonf: i think you want an attribute that means on first load only
<jarhar> masonf: other thing is that timing is a bit tricky. you have to precisely define the timing. if you have two things that are initially modal you have to decide what order they go in, what if theres a script in the middle, etc.
<masonf> ack mason
<masonf> ack brecht
<jarhar> brecht_dr: i do come in a lot of situations where i need this
<jarhar> brecht_dr: for example, a user agreement has changed for a certain platform when a user logs in
<jarhar> brecht_dr: you want to show a modal right away that says this information
<jarhar> brecht_dr: another one is ?? on platforms, not sure if this is a modal, but is a use case
<jarhar> brecht_dr: not sure if cookie banner should be a modal
<jarhar> brecht_dr: something thats also open by default when the page loads
<Luke> q+
<jarhar> brecht_dr: theres certainly a big use case for these kind of things
<jarhar> brecht_dr: already had the same thing for popovers as well
<jarhar> brecht_dr: should be something thats out there
<jarhar> brecht_dr: i do understand theres a lot of trickiness involved
<jarhar> brecht_dr: especially when loading by default, the last thing in your dom that has open will be open in the end
<jarhar> brecht_dr: in favor of having something like this
<masonf> jarhar: If we get rid of non-modal dialogs, we don't need a modal attribute any more?
<scotto_> q+
<masonf> jarhar: if not, then I agree with defaultopen - open when the page loads. No tracking of current state.
<masonf> jarhar: Original spec PR had defaultopen and we removed it - might be hard to spec.
<jarhar> masonf: defaultopen - there were a lot of open questions so we removed it
<jarhar> Luke: two points of clarification: i would foresee this as not being removed, so the attribute would always exist on the dialog
<jarhar> Luke: modal would just indicate that this is a modal dialog
<keithamus> q+
<jarhar> Luke: if you want something thats modal, its not also going to want to be non-modal at some time
<jarhar> Luke: regarding making open just mean open by default isnt plausible
<jarhar> Luke: for backwards compat reasons
<jarhar> Luke: deprecating the show function doesnt...
<jarhar> Luke: depends on meaning of deprecation
<jarhar> Luke: if its a thing thats deprecated then you should use popover but you cant get rid of it
<jarhar> Luke: but yeah potentially it is a thing - if we get rid of non-modal dialogs then open without popover means modal, and open with popover means non-modal
<jarhar> Luke: also solves invoker thing because youd use modal actions or popover actions
<masonf> ack jarhar
<masonf> ack luke
<jarhar> Luke: to me the modal attribute would stick around
<masonf> ack scott
<jarhar> scotto_: touching on what we were talking about with modals and show, people still need non-modal dialogs and they shjouldnt go away - its just how are they shown - popovers?
<jarhar> scotto_: .show() thats going away, not non-modal
<jarhar> scotto_: dialogs cant be modal by default, you dont have to put a close button in them. someone could show one and now you cant use anything because you dont have access to an escape key if youre on mobile
<jarhar> scotto_: if dialog is modal by default, youre going to need some kind of invoking action - the thing is just there and the entire page is inert
<jarhar> scotto_: this needs to be something that someone explicitly does, you cant just make a dialog modal by default
<jarhar> scotto_: this would break those pages, there doesnt have to be a close button so how are people going to close these
<jarhar> Luke: mobile platforms do have back gestures
<brecht_dr> q+
<jarhar> scotto_: i dont like the back gesture thing
<jarhar> masonf: its shipping
<jarhar> scotto_: i hate it
<masonf> ack keith
<jarhar> keithamus: i want to say -1 to this based on everything. theres a lot of complexity in the spec and what mason mentioned around timing, and user complexity
<jarhar> keithamus: luke in the comment you mentioned dialog modal open is invalid and an authoring error
<jarhar> keithamus: im sure that people have examples
<jarhar> keithamus: to scotts point, im concerned about user interaction
<jarhar> keithamus: if you want to present someone with a modal dialog on page load, thats a page. a modal dialog is like a mini page, and its alleviating page navigations
<jarhar> keithamus: if youre introducing that, then thats just the page
<masonf> keep in mind, you can currently do <script>window.onload = () => mydialog.showModal()</script>
<jarhar> keithamus: i struggle to see a compelling use case for that as well
<jarhar> keithamus: im interested if there are any
<brecht_dr> q-
<jarhar> Luke: to keiths point, yeah it is bad but a design question as whether you want something to be a modal or its own page
<jarhar> Luke: if you have a data table and are renaming something, its nice to have it be a small modal rather than another page
<jarhar> Luke: not necessarily a good design, but a design question
<jarhar> Luke: if we have the modal attribute, it would simplify the toggles
<jarhar> Luke: invoker would decide whether its modal
<jarhar> keithamus: im wondering if the justification is in reverse. better to focus energy on do we need .show() and then the two types of dialogs are showmodal with script or invoker, or you have a dialog popover, and those are the two
<jarhar> keithamus: right now there are three and its making this complicated
<jarhar> keithamus: stepping further into adding another thing to increase complexity might not solve the problem
<jarhar> masonf: on the invokers - complexity is around toggle and close. auto makes sense and is modal
<jarhar> masonf: so maybe a way around this is to pull out the toggle stuff from the invokers and punt this down the road
<jarhar> keithamus: i think we have that conversation coming up
<jarhar> Luke: im happy to leave this and move on to the next one
<jarhar> masonf: i didnt hear consensus, is there a resolution?
<jarhar> Luke: for now, if you have opinions then leave them on the issue
<jarhar> Luke: dont have a strong resolution to go with

@lukewarlow lukewarlow removed the agenda+ Use this label if you'd like the topic to be added to the meeting agenda label Nov 9, 2023
Copy link

github-actions bot commented May 8, 2024

There hasn't been any discussion on this issue for a while, so we're marking it as stale. If you choose to kick off the discussion again, we'll remove the 'stale' label.

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

No branches or pull requests

5 participants