jackbsteinberg / std-toast

120 stars 4 forks source link

Close button API #48

Open fergald opened 5 years ago

fergald commented 5 years ago

Maybe this should just be in #47 but it's not just about naming.

Is a text attribute the right approach? Should we support e.g. an image for a close button? (Edit, so e.g. a slotted child).

Also, the spec for the value of the closeButton attribute seems a bit complicated, it's trying to combine 2 things into 1

and I'm not sure that's a great idea. We could consider having a closeable boolean property and a closebutton that returns a string. I don't know if that's really better but a property that can true/false/string seems very odd to me.

tkent-google commented 5 years ago

The situation is similar to value attribute/property of <input type=submit>. It works as:

tkent-google commented 5 years ago

IDL attributes for accessibility attributes apply nullable DOMString.

https://w3c.github.io/aria/#idl-interface

domenic commented 5 years ago

I agree the API is a bit weird. It is favoring developer ergonomics over platform consistency at the moment. I am not sure we've landed on the right part of the spectrum there so it's good to have an issue to discuss.

To expand more on why I think the current design is nice developer ergonomics, it basically allows developers to operate in one of two mindsets, without caring which:

Current explainer

<std-toast closebutton>...</std-toast>
<std-toast closebutton="Dismiss">...</std-toast>
/* BOOLEAN MODE */

toast.closeButton = true;
toast.closeButton = false;

// This works even if other parts of the code set closeButton to a string
if (toast.closeButton) { ... }

showToast("Message", { closeButton: true });

/* STRING MODE */

toast.closeButton = "Dismiss";

if (toast.closeButton === "Dismiss") { ... }

showToast("Message", { closeButton: "Dismiss" });

A couple alternatives have been mentioned in the thread so far:

Nullable DOMString

Here if the attribute is missing, closeButton is null (and the close button is not shown). If the attribute is present with the empty string value, then it has the default close button contents (I think?). And if the attribute is present with another value, that is the close button contents. This looks like:

<std-toast closebutton>...</std-toast>
<std-toast closebutton="Dismiss">...</std-toast>
toast.closeButton = ""; // set default close button
toast.closeButton = null; // remove close button
toast.closeButton = "Dismiss";

if (toast.closeButton !== null) { ... }

showToast("Message", { closeButton: "" });
showToast("Message", { closeButton: "Dismiss" });

(Note: I assume we keep the showToast option the same as the property, but we could also explore diverging them. I'll keep it simple for now.)

Two separate attributes

This looks like:

<std-toast closeable>...</std-toast>
<std-toast closeable closebutton="Dismiss">...</std-toast>

<!-- does nothing useful -->
<std-toast closebutton="Dismiss">...</std-toast>
// set default close button
toast.closeable = true;

// set customized close button
toast.closeable = true;
toast.closeButton = "Dismiss";

// remove close button
toast.closeable = false;

// don't do this, it will make a close button with no text/the string "null" inside:
toast.closeButton = "";
toast.closeButton = null;

if (toast.closable) { ... }

// We could probably make closeable: true implicit
// if closeButton is specified for showToast():
showToast("Message", { closable: true });
showToast("Message", { closeButton: "Dismiss" });

Thoughts welcome!

fergald commented 5 years ago

There's also

<std-toast>
  Content
  <img src="..." slot=closebutton>
</std-toast>