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

Remove alert() from live samples #7566

Open
15 of 27 tasks
wbamberg opened this issue Aug 3, 2021 · 12 comments
Open
15 of 27 tasks

Remove alert() from live samples #7566

wbamberg opened this issue Aug 3, 2021 · 12 comments
Labels
MDN:Project Anything related to larger core projects on MDN

Comments

@wbamberg
Copy link
Collaborator

wbamberg commented Aug 3, 2021

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.

See also #7553.

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:

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 wbamberg added the needs triage Triage needed by staff and/or partners. Automatically applied when an issue is opened. label Aug 3, 2021
@wbamberg

This comment was marked as outdated.

@advitiya7
Copy link
Contributor

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

@teoli2003
Copy link
Contributor

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

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

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(): Using console.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.

@Josh-Cena Josh-Cena added the MDN:Project Anything related to larger core projects on MDN label Jun 27, 2023
@evelinabe
Copy link
Contributor

evelinabe commented Jun 27, 2024

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

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

@hamishwillee
Copy link
Collaborator

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

This comment was marked as duplicate.

@evelinabe
Copy link
Contributor

evelinabe commented Jul 16, 2024

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

Josh-Cena commented Jul 16, 2024

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

bsmth commented Jul 17, 2024

That was me in #32258 which fixed var etc., but didn't catch alert() uses. Good idea about <dialog>, I think we can use that instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MDN:Project Anything related to larger core projects on MDN
Projects
None yet
Development

No branches or pull requests

8 participants