Remove alert() from live samples

wbamberg commented 3 years ago

window.alert() doesn't work cross-origin in Chrome any more, meaning that any of our live samples that rely on alert() are now broken.

This is (I think) all the live samples in Web/API that use alert() in a live sample (list now maintained by @Josh-Cena):

(I might have missed some).

In addition, the following ones use prompt() and are also broken:

I just realized we have some prompt too, so I'm adding them to this list:

We should replace all of these with some alternative. A common alternative is to write the output into an element in the example's document.

wbamberg commented 3 years ago

This is the complete list of pages under Web/API that contain a <pre class="js" block that contains alert(:

We might want to replace these as well, but they are not actually broken unless they're in a live sample.

advitiya7 commented 1 year ago

hey can you help me understand the problem i would like to contribute to this thanks

teoli2003 commented 1 year ago

On two of the remaining pages, there are live example using alert(), like this one: https://developer.mozilla.org/en-US/docs/Web/API/eventtarget/addeventlistener#example_of_options_usage

Such example are bad practice (we should not use alert() and are not working on some browsers.

The goal is to replace them using a log element that we use to display the alert text instead.

The numerous merged PRs show many examples of such conversion.

advitiya7 commented 1 year ago

thanks for the help @teoli2003 so basically i have to replace alert() with console.log right? and for clear alert() i should do clear message() sorry if this seems silly to you it's my first time and i want it to be right.

teoli2003 commented 1 year ago

No, it is more complex than that. You need to add an element, like <pre id="log">, and write inside it (using textContent) instead of using alert(): Usingconsole.log()` is not enough as the log will be hidden by default.

For clearing the message, you simply set a new value with textContent.

Of course, positioning the message box may be tricky, and it is likely you'll need to expand the size in {{EmbedLiveSample}} so that no scrollbar appears.

evelinabe commented 1 month ago

I took a look at this one MDN/Writing_guidelines/Page_structures/Live_samples Since the intention of the section is to show a simple example of a JavaScript function, I get the feeling that an alternative to alert() will be too complex. I was thinking about replacing the alert with manipulating the text inside the div on click instead, what would you think of something like this?

It's a bit more complex than the original, but still a simple interaction that keeps most elements in the example as is.

const el = document.getElementById("item")

let toggleClick = false

el.onclick = function () {
  if (!toggleClick) {
    this.textContent = "Owww, stop poking me!"
    toggleClick = true
  } else {
    this.textContent = "Hello world! Welcome to MDN"
    toggleClick = false
  }
}

Josh-Cena commented 1 month ago

Yeah, something like that is what we are looking for.

hamishwillee commented 1 month ago

The https://developer.mozilla.org/en-US/docs/MDN/Writing_guidelines/Page_structures/Live_samples#displaying_a_log_that_appends_items has two examples of how you can create a log to use instead of an alert - one that displays a single "current value" and the other than appends results. This can replace an alert of console.log() example in most cases

Josh-Cena commented 1 month ago

I just realized we have some prompt too, so I'm adding them to this list:

evelinabe commented 1 month ago

The first six files in the list belong to the same tutorial, so they should have the same solution. The alert is used to display the message "Game Over". Would you prefer the simple solution of adding a log below the button with the message or expanding Games/Tutorials/2D_Breakout_game_pure_JavaScript/Game_over somewhat with a gameOverfunction that shows the message on an overlay on the game div for a couple of seconds before reload?

Josh-Cena commented 1 month ago

I'm not very sure. They were brought into live examples by @bsmth, so perhaps he has some ideas here? I think using a <dialog> is the right approach.

bsmth commented 1 month ago

That was me in https://github.com/mdn/content/pull/32258 which fixed var etc., but didn't catch alert() uses. Good idea about <dialog>, I think we can use that instead.

mdn / content

Remove alert() from live samples #7566