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

Close button API #48

Open
fergald opened this issue Jul 17, 2019 · 4 comments
Open

Close button API #48

fergald opened this issue Jul 17, 2019 · 4 comments

Comments

@fergald
Copy link
Collaborator

fergald commented Jul 17, 2019

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

  • is there a closebutton?
  • what do we display for a close button?

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
Copy link

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

  • If value attribute is missing, show the default label like "Submit".
  • If value attribute is specified, show the value attribute value even if it's empty.
  • value property reflects value attribute. It returns "" for both of missing attribute and an empty attribute.

@tkent-google
Copy link

IDL attributes for accessibility attributes apply nullable DOMString.

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

@domenic
Copy link
Collaborator

domenic commented Jul 17, 2019

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!

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 17, 2019
…se functionality

This change sets the groundwork for the toast closebutton, by
adding an attribute to set the closebutton content,
creating a closeButton property to reflect that attribute,
and adding an event listener to close the toast when the closebutton is pressed.

This change implements the close button API described in
(https://github.com/jackbsteinberg/std-toast/blob/eec7728f7082a897d777181ac07b0448062ffca5/README.md),
and is the subject of debate on issue #48 on the explainer here
jackbsteinberg/std-toast#48.

Future CLs will add the closeButton option for showToast
(https://github.com/jackbsteinberg/std-toast/tree/master#showtoastmessage-options)
Link: https://chromium-review.googlesource.com/c/chromium/src/+/1706453

Bug:972945
Change-Id: I5d6c3055791a9f93ef414e575e128c61e2a944ed
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 18, 2019
…se functionality

This change sets the groundwork for the toast closebutton, by
adding an attribute to set the closebutton content,
creating a closeButton property to reflect that attribute,
and adding an event listener to close the toast when the closebutton is pressed.

This change implements the close button API described in
(https://github.com/jackbsteinberg/std-toast/blob/eec7728f7082a897d777181ac07b0448062ffca5/README.md),
and is the subject of debate on issue #48 on the explainer here
jackbsteinberg/std-toast#48.

Future CLs will add the closeButton option for showToast
(https://github.com/jackbsteinberg/std-toast/tree/master#showtoastmessage-options)
Link: https://chromium-review.googlesource.com/c/chromium/src/+/1706453

Bug: 972945
Change-Id: I5d6c3055791a9f93ef414e575e128c61e2a944ed
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1699269
Reviewed-by: Fergal Daly <fergal@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Jack Steinberg <jacksteinberg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#678530}
aarongable pushed a commit to chromium/chromium that referenced this issue Jul 18, 2019
…se functionality

This change sets the groundwork for the toast closebutton, by
adding an attribute to set the closebutton content,
creating a closeButton property to reflect that attribute,
and adding an event listener to close the toast when the closebutton is pressed.

This change implements the close button API described in
(https://github.com/jackbsteinberg/std-toast/blob/eec7728f7082a897d777181ac07b0448062ffca5/README.md),
and is the subject of debate on issue #48 on the explainer here
jackbsteinberg/std-toast#48.

Future CLs will add the closeButton option for showToast
(https://github.com/jackbsteinberg/std-toast/tree/master#showtoastmessage-options)
Link: https://chromium-review.googlesource.com/c/chromium/src/+/1706453

Bug: 972945
Change-Id: I5d6c3055791a9f93ef414e575e128c61e2a944ed
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1699269
Reviewed-by: Fergal Daly <fergal@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Jack Steinberg <jacksteinberg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#678530}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 18, 2019
…se functionality

This change sets the groundwork for the toast closebutton, by
adding an attribute to set the closebutton content,
creating a closeButton property to reflect that attribute,
and adding an event listener to close the toast when the closebutton is pressed.

This change implements the close button API described in
(https://github.com/jackbsteinberg/std-toast/blob/eec7728f7082a897d777181ac07b0448062ffca5/README.md),
and is the subject of debate on issue #48 on the explainer here
jackbsteinberg/std-toast#48.

Future CLs will add the closeButton option for showToast
(https://github.com/jackbsteinberg/std-toast/tree/master#showtoastmessage-options)
Link: https://chromium-review.googlesource.com/c/chromium/src/+/1706453

Bug: 972945
Change-Id: I5d6c3055791a9f93ef414e575e128c61e2a944ed
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1699269
Reviewed-by: Fergal Daly <fergal@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Jack Steinberg <jacksteinberg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#678530}
@fergald
Copy link
Collaborator Author

fergald commented Jul 18, 2019

There's also

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

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jul 24, 2019
…st closebutton and attach close functionality, a=testonly

Automatic update from web-platform-tests
Create an attribute and property for toast closebutton and attach close functionality

This change sets the groundwork for the toast closebutton, by
adding an attribute to set the closebutton content,
creating a closeButton property to reflect that attribute,
and adding an event listener to close the toast when the closebutton is pressed.

This change implements the close button API described in
(https://github.com/jackbsteinberg/std-toast/blob/eec7728f7082a897d777181ac07b0448062ffca5/README.md),
and is the subject of debate on issue #48 on the explainer here
jackbsteinberg/std-toast#48.

Future CLs will add the closeButton option for showToast
(https://github.com/jackbsteinberg/std-toast/tree/master#showtoastmessage-options)
Link: https://chromium-review.googlesource.com/c/chromium/src/+/1706453

Bug: 972945
Change-Id: I5d6c3055791a9f93ef414e575e128c61e2a944ed
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1699269
Reviewed-by: Fergal Daly <fergal@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Jack Steinberg <jacksteinberg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#678530}

--

wpt-commits: bd0b2bff5361391e8c25949aa40f3167dd82d572
wpt-pr: 17869
xeonchen pushed a commit to xeonchen/gecko that referenced this issue Jul 25, 2019
…st closebutton and attach close functionality, a=testonly

Automatic update from web-platform-tests
Create an attribute and property for toast closebutton and attach close functionality

This change sets the groundwork for the toast closebutton, by
adding an attribute to set the closebutton content,
creating a closeButton property to reflect that attribute,
and adding an event listener to close the toast when the closebutton is pressed.

This change implements the close button API described in
(https://github.com/jackbsteinberg/std-toast/blob/eec7728f7082a897d777181ac07b0448062ffca5/README.md),
and is the subject of debate on issue #48 on the explainer here
jackbsteinberg/std-toast#48.

Future CLs will add the closeButton option for showToast
(https://github.com/jackbsteinberg/std-toast/tree/master#showtoastmessage-options)
Link: https://chromium-review.googlesource.com/c/chromium/src/+/1706453

Bug: 972945
Change-Id: I5d6c3055791a9f93ef414e575e128c61e2a944ed
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1699269
Reviewed-by: Fergal Daly <fergal@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Jack Steinberg <jacksteinberg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#678530}

--

wpt-commits: bd0b2bff5361391e8c25949aa40f3167dd82d572
wpt-pr: 17869
natechapin pushed a commit to natechapin/wpt that referenced this issue Aug 23, 2019
…se functionality

This change sets the groundwork for the toast closebutton, by
adding an attribute to set the closebutton content,
creating a closeButton property to reflect that attribute,
and adding an event listener to close the toast when the closebutton is pressed.

This change implements the close button API described in
(https://github.com/jackbsteinberg/std-toast/blob/eec7728f7082a897d777181ac07b0448062ffca5/README.md),
and is the subject of debate on issue web-platform-tests#48 on the explainer here
jackbsteinberg/std-toast#48.

Future CLs will add the closeButton option for showToast
(https://github.com/jackbsteinberg/std-toast/tree/master#showtoastmessage-options)
Link: https://chromium-review.googlesource.com/c/chromium/src/+/1706453

Bug: 972945
Change-Id: I5d6c3055791a9f93ef414e575e128c61e2a944ed
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1699269
Reviewed-by: Fergal Daly <fergal@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Jack Steinberg <jacksteinberg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#678530}
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 4, 2019
…st closebutton and attach close functionality, a=testonly

Automatic update from web-platform-tests
Create an attribute and property for toast closebutton and attach close functionality

This change sets the groundwork for the toast closebutton, by
adding an attribute to set the closebutton content,
creating a closeButton property to reflect that attribute,
and adding an event listener to close the toast when the closebutton is pressed.

This change implements the close button API described in
(https://github.com/jackbsteinberg/std-toast/blob/eec7728f7082a897d777181ac07b0448062ffca5/README.md),
and is the subject of debate on issue #48 on the explainer here
jackbsteinberg/std-toast#48.

Future CLs will add the closeButton option for showToast
(https://github.com/jackbsteinberg/std-toast/tree/master#showtoastmessage-options)
Link: https://chromium-review.googlesource.com/c/chromium/src/+/1706453

Bug: 972945
Change-Id: I5d6c3055791a9f93ef414e575e128c61e2a944ed
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1699269
Reviewed-by: Fergal Daly <fergalchromium.org>
Reviewed-by: Kent Tamura <tkentchromium.org>
Commit-Queue: Jack Steinberg <jacksteinbergchromium.org>
Cr-Commit-Position: refs/heads/master{#678530}

--

wpt-commits: bd0b2bff5361391e8c25949aa40f3167dd82d572
wpt-pr: 17869

UltraBlame original commit: 8930d78b9176fc46cc05bc4f0f3f1465128d1da1
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 4, 2019
…st closebutton and attach close functionality, a=testonly

Automatic update from web-platform-tests
Create an attribute and property for toast closebutton and attach close functionality

This change sets the groundwork for the toast closebutton, by
adding an attribute to set the closebutton content,
creating a closeButton property to reflect that attribute,
and adding an event listener to close the toast when the closebutton is pressed.

This change implements the close button API described in
(https://github.com/jackbsteinberg/std-toast/blob/eec7728f7082a897d777181ac07b0448062ffca5/README.md),
and is the subject of debate on issue #48 on the explainer here
jackbsteinberg/std-toast#48.

Future CLs will add the closeButton option for showToast
(https://github.com/jackbsteinberg/std-toast/tree/master#showtoastmessage-options)
Link: https://chromium-review.googlesource.com/c/chromium/src/+/1706453

Bug: 972945
Change-Id: I5d6c3055791a9f93ef414e575e128c61e2a944ed
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1699269
Reviewed-by: Fergal Daly <fergalchromium.org>
Reviewed-by: Kent Tamura <tkentchromium.org>
Commit-Queue: Jack Steinberg <jacksteinbergchromium.org>
Cr-Commit-Position: refs/heads/master{#678530}

--

wpt-commits: bd0b2bff5361391e8c25949aa40f3167dd82d572
wpt-pr: 17869

UltraBlame original commit: 8930d78b9176fc46cc05bc4f0f3f1465128d1da1
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 4, 2019
…st closebutton and attach close functionality, a=testonly

Automatic update from web-platform-tests
Create an attribute and property for toast closebutton and attach close functionality

This change sets the groundwork for the toast closebutton, by
adding an attribute to set the closebutton content,
creating a closeButton property to reflect that attribute,
and adding an event listener to close the toast when the closebutton is pressed.

This change implements the close button API described in
(https://github.com/jackbsteinberg/std-toast/blob/eec7728f7082a897d777181ac07b0448062ffca5/README.md),
and is the subject of debate on issue #48 on the explainer here
jackbsteinberg/std-toast#48.

Future CLs will add the closeButton option for showToast
(https://github.com/jackbsteinberg/std-toast/tree/master#showtoastmessage-options)
Link: https://chromium-review.googlesource.com/c/chromium/src/+/1706453

Bug: 972945
Change-Id: I5d6c3055791a9f93ef414e575e128c61e2a944ed
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1699269
Reviewed-by: Fergal Daly <fergalchromium.org>
Reviewed-by: Kent Tamura <tkentchromium.org>
Commit-Queue: Jack Steinberg <jacksteinbergchromium.org>
Cr-Commit-Position: refs/heads/master{#678530}

--

wpt-commits: bd0b2bff5361391e8c25949aa40f3167dd82d572
wpt-pr: 17869

UltraBlame original commit: 8930d78b9176fc46cc05bc4f0f3f1465128d1da1
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

3 participants