openui / open-ui

Maintain an open standard for UI and promote its adherence and adoption.
https://open-ui.org
Other
3.59k stars 191 forks source link

[invokers] Dialog's `toggle` needs a better name #956

Open keithamus opened 12 months ago

keithamus commented 12 months ago

Invokers targeting a <dialog> can have invokeaction=toggle, which will open and close the dialog as non-modal.

toggle is a little ambiguous, especially as all other names are very clear in their action, togglePopover, playpause, etc.

Given https://github.com/whatwg/html/issues/9376 (Should we deprecate Dialog show()?) perhaps we can simply not implement this?

If we are to implement this, it needs a clearer name. Some bad ideas to start the bidding:

lukewarlow commented 12 months ago

I vote "don't implement". ToggleOpen isn't a perfect representation of what it does (given focusing) so I would vote against that.

css-meeting-bot commented 11 months ago

The Open UI Community Group just discussed [invokers] Dialog's `toggle` needs a better name, and agreed to the following:

The full IRC log of that discussion <masonf> q?
<hdv> keithamus: the toggle method may be ambiguous… the others are unambiguous to the target element, like showPopover
<Luke> q+
<masonf> q+
<hdv> keithamus: for showing and hiding a dialog for an invoke button
<masonf> q+ sasha
<hdv> Luke: details is ambiguous as well
<hdv> Luke: having said that, details isn't in the v1
<hdv> Luke: would say don't implement show() or toggle() in the initial version
<hdv> s/show()/show
<hdv> s/toggle()/toggle
<masonf> ack luke
<hdv> Luke: so if we decide to keep non modal dialog in the future for whatever reason ew could
<hdv> Luke: most people will likely use a modal dialog or a popover
<hdv> masonf: can we punt the whole toggle thing… there is no such API on dialogs now can we just leave showmodal and close and auto?
<hdv> masonf: I'm still questioning… if you're toggling… the use case for toggle on a modal doens't make sense to me
<masonf> ack mason
<hdv> keithamus: use case for toggle on modal is the auto action
<hdv> keithamus: do we have a name mapping for the auto action other than auto?
<hdv> keithamus: another meta question around this… do we intentionally want tot disincentivise showing non modal dialog from dialog?
<hdv> s/tot/to
<hdv> masonf: don't think we should actively disincentivise anything
<hdv> masonf: on most parts of the platform auto is magic
<keithamus> q+
<hdv> masonf: I don't think there's a problem with auto not mapping to a string value you can provide
<masonf> ack sasha
<hdv> sasha: I can see the reference to existing solutions here
<hdv> sasha: I think we need to align if it fits with the old semantics… it looks from this convo that it does, only want to display or hide
<hdv> sasha: not sure about the toggle as a part of the flow in the page… they usually work together with the whole page… what would happen next after triggering the dialog?
<masonf> q?
<hdv> sasha: when we do the proposal it would be nice to outline generic use cases and scope
<masonf> ack keith
<hdv> keithamus: one reason not to want auto this way is that auto is ambiguous depending on the target element
<hdv> keithamus: I didn't understand your last point sasha?
<hdv> sasha: usually they are triggering the visibility associated with the next steps in the user flow
<hdv> sasha: so if you toggle a dialog and dialog has an extra attribute, they would not just toggle visibility of that one, but also toggle details with open attribute after the close of this dialog
<hdv> keithamus: I think dialogs already have some of the behaviour that you describe
<hdv> masonf: there are still events, close and cancel on dailogs, that could be used to trigger the next
<hdv> sasha: am thinking as declarative
<masonf> q?
<scotto_> q+
<hdv> luke: on toggle button you have buttons that are literally toggles… but on a modal you'd have buttons that only do either, just a single action
<masonf> q+
<hdv> Luke: so I think folks would either go auto or toggle mode
<keithamus> q+
<hdv> scotto_: +1 what you mentioned Mason, Keith and Luke both know I've had a weirdness to toggling modals, you can't do that from a single button, outside of a weird use case of someone forcing a dialog to be open
<hdv> scotto_: so we probably don't need to add a toggle attribute for that
<hdv> ack scotto_
<hdv> masonf: not having auto for dialogs feels kind of funny
<hdv> masonf: so the spec would have to say if it is a popover, do this, if it's a dialog, do that… so maybe auto doesn't need to mean toggle, it can mean use as modal?
<masonf> ack masonf
<hdv> Luke: we don't have auto for everything, eg for select there isn't an auto
<hdv> masonf: shoulnd't there be?
<hdv> Luke: if we add togglepicker or closepicker… if we have auto now we couldn't add those in the future
<hdv> masonf: I'd argue show is a good default there for same reason as dialog
<hdv> Luke: I see the point
<hdv> q?
<masonf> q+ charlie
<hdv> keithamus: the intend of auto toggling the modal is thatyou don't have to specify the invokeaction attribute
<hdv> keithamus: so inside the dialog not specifying the attribute would do the sensible thing, close the attribute… it saves devs from writing the attribute, browser could be clever enough to do the right thing
<hdv> keithamus: maybe the correct thing to do here… remove the aliases to auto, and auto will, as spec says today, if target el is popover, it will toggle as a popover, if the element is a dialog, it will show as modal unless dialog is open then it willl close it
<masonf> q?
<masonf> ack keith
<masonf> ack charlie
<hdv> charlie: my concern with going ahead with auto, because we don't know the full list of invokee elements right now, but want to support it for some other than popover and dialog, we probably want to keep it consistent
<hdv> charlie: this may give us confusing use cases
<masonf> q+ sasha
<hdv> charlie: my vote would be to defer auto until we have a better idea what would be included in invokers
<hdv> masonf: so you say it would feel weird for auto to do fundamentally different things for different elements so it is fair to wait until we know more?
<hdv> charlie: yes
<hdv> masonf: would cause for everyone to always add both attributes
<hdv> Luke: is the case for popover currently too
<hdv> masonf: it is nice not to have to add extra attributes
<hdv> sasha: some buttons could have three states, sounds like these could be subject to the mode, would be nice to provide those states
<masonf> q?
<hdv> masonf: top layer is pretty carefully controlled so you can't change popover to dialog without closing it first
<keithamus> PROPOSED RESOLUTION: Defer toggle, and remove `auto` aliases (such as `toggleModal`).
<masonf> ack sasha
<hdv> masonf: maybe to clarify: do you mean continue supporting auto but not having a mapping?
<hdv> keithamus: yes
<hdv> Luke: you intend to keep togglePopover as a standalone?
<hdv> masonf: that's pretty well defined, maps to a function
<keithamus> Proposed Solution: Defer `toggle`, remove the explicit `toggleModal` string, and keep `auto` behaviours as is.
<masonf> +1
<scotto_> +1
<hdv> +1
<hdv> Luke: can I clarify does this include show?
<hdv> masonf: would make sense to keep?
<keithamus> Proposed Resolution: Defer `toggle` and `show`, remove the explicit `toggleModal` string, and keep `auto` behaviours as is.
<masonf> +1
<Luke> +1
<hdv> +2
<keithamus> Resolved: Defer `toggle` and `show`, remove the explicit `toggleModal` string, and keep `auto` behaviours as is.
<hdv> (for British spelling)
<keithamus> RESOLVED: Defer `toggle` and `show`, remove the explicit `toggleModal` string, and keep `auto` behaviours as is.
sashafirsov commented 11 months ago

The separation of concerns IMO needs separate attributes:

It is important as provide each functionality separately and do not mix those. As the imperative as declarative syntax for all. In this case declarative can be limited by allocation of DIALOG attributes.

Open

open as name for visibility to match the other HTML elements like in details element

Toggle/Trigger

triggering the next UI on the toggle as pointer to the what should happen to element refered say by for attribute. Scenario:

Mode

the modal vs popover vs inline is rather the subject for mode attribute of Dialog.

github-actions[bot] commented 5 months ago

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.