Code samples are duplicated for viewing vs executing

[zenflow]

Code samples are duplicated for viewing vs executing

[zenflow]

As the core maintainer, I realize the limited amount of my resources and contributors' resources and I will be ruthless to everything which isn't important, i.e. opinionated feature requests on internal things.

I don't think it's very opinionated, I think it's a pretty objective (though relatively minor) issue. But being a minor issue doesn't mean it's a non-issue that should be closed to say it should not be fixed.

What you're doing here @zenflow is asking me (because nobody else will probably do that) to implement the feature I disagree about. That doesn't make much sense.

I'm not asking you to do it @limonte. I was not asking anyone to do it. I was just pointing out that it is still an issue. I was thinking of doing it .. the only problem is that the page is old-school static html, and dynamic rending happens only in the browser.. so although we could populate the <pre> elements on page load, the contents may not be visible to all web crawlers. If that was an acceptable trade-off (since I believe Google will still pick up that content?) I could easily fix this issue.

Also, I thought it would be easier to reason with you and change the fact that you disagree with this issue.

It seems that you are inclined to see this as a non-issue because you don't see an easy solution to it? But not having an easy solution doesn't/wouldn't in reality make it any less of an issue.

I really think that we might want to learn to prioritize and spend our valuable time on tasks with higher priority which affect actual functionality and UX of SweetAlert2 and its React, Angular plugins.

It's the principle, which applies in more than just this situation: we shouldn't close issues without valid reasons. In this case there have been mostly invalid reasons.

[zenflow]

It is totally possible to execute the example code inside an async function, while displaying the same example code without an async function.

Could you please elaborate on this statement with an example, apparently I don't know something you know about JS

I don't think you're missing any knowledge of JS here.. This is what I mean:

It's possible to have the example code inside an async function and display the same example code without an async function.

[zenflow]

Part of #1

[limonte]

I'm marking this issue as wontfix because there are some cases when we don't want the actual code and the representation code to be the same.

E.g. RTL example. The actual code is using target: document.getElementById('rtl-container') but I don't want to show that in the representation code because some people will think it's necessary to use custom container to enable RTL support when in fact it's not true.

[zenflow]

E.g. RTL example. The actual code is using target: document.getElementById('rtl-container') but I don't want to show that in the representation code because some people will think it's necessary to use custom container to enable RTL support when in fact it's not true.

@limonte I think then this is just not a good example. and should be fixed to be a good example.

Users are expecting the code executed to be the same as the code displayed, and it will be confusing otherwise, so we should always have them be the same, not just similar.

Please reopen if you agree.

Regarding that particular example, I think we can include the potentially-confusing code, along with a code comment noting why it's there and that it's not necessary.

[zenflow]

@limonte rather, re-close if you disagree

[limonte]

I do disagree. Here's another example which applies to a lot of examples with await keywords.

On the docs page you can see the simple and beautiful example:

const {value: email} = await Swal.fire({
  title: 'Input email address',
  input: 'email',
  inputPlaceholder: 'Enter your email address'
})

if (email) {
  Swal.fire('Entered email: ' + email)
}

This example is not copy-paste friendly because await can be used only in async functions, I understand that. But we're not teaching JS, we're showing usage examples of SweetAlert2.

Adding IIFE to async/await examples

(async function getEmail () { 
  const {value: email} = await Swal.fire({
    title: 'Input email address',
    input: 'email',
    inputPlaceholder: 'Enter your email address'
  })

  if (email) {
    Swal.fire('Entered email: ' + email)
  }
})()

will decrease the readability of examples, which is the opposite of my intentions.

[zenflow]

@limonte Regarding this second example: The example code is what's inside the async function. You can put the example code inside an async function to execute it, (similar to how we put the example code inside a <pre> element to display it) and that doesn't change the example code. It is totally possible to execute the example code inside an async function, while displaying the same example code without an async function.

This second example is very different from the first example of actually having two different versions of the example code. I really don't know how you could think this is a good idea, since "example code displayed does not match example code executed" seems to be an obvious and self-explanatory issue, and a bigger issue than this issue. It will be confusing to the users until they figure this out, at which point it will be a PITA since it's not easy for them to know with certainty (1) what code was executed when they clicked the button (since the same code is not displayed), or (2) what will happen when the displayed example code is executed (since the same code is not executed when they click the button).

[zenflow]

@limonte If you still think it's a good idea to have two different versions of the example code (one version for displaying and one version for executing) then I suggest you get @gverni's opinion here

[limonte]

It is totally possible to execute the example code inside an async function, while displaying the same example code without an async function.

Could you please elaborate on this statement with an example, apparently I don't know something you know about JS.

[limonte]

As the core maintainer, I realize the limited amount of my resources and contributors' resources and I will be ruthless to everything which isn't important, i.e. opinionated feature requests on internal things.

What you're doing here @zenflow is asking me (because nobody else will probably do that) to implement the feature I disagree about. That doesn't make much sense.

I really think that we might want to learn to prioritize and spend our valuable time on tasks with higher priority which affect actual functionality and UX of SweetAlert2 and its React, Angular plugins.

[limonte]

But being a minor issue doesn't mean it's a non-issue that should be closed to say it should not be fixed.

That's correct. To be honest, my reason for closing was "I don't know how to do this" which can't be considered as a valid reason indeed.

I'm reopening this issue, RTL example could me moved to https://github.com/sweetalert2/sweetalert2-examples and async/await examples can be apparently represented without IIFE wrappers.

[limonte]

PS. thanks @zenflow for standing for this issue. Deep inside, I'd like to get it done, but my lazy part closed it purely because of laziness to figure out the way to fix it.

I'd like to get some help in the prototyping of this:

[zenflow]

I'm reopening this issue, RTL example could me moved to https://github.com/sweetalert2/sweetalert2-examples and async/await examples can be apparently represented without IIFE wrappers.

Why move the example? Also gotta ask: why not have all the examples in the documentation site? (i.e. Why have a separate site for examples?)

[zenflow]

I'd like to get some help in the prototyping of this:

I was looking into whether it's feasible make the change to build the index.html, with the code sample <pre> elements fully rendered, to be able to solve this issue without losing the <pre> element content from the static index.html and just populating it on pageload. (See the "problem" mentioned in #27 (comment) 2nd paragraph/response). Let me know if it's not clear what I'm talking about.

But I found a problem that is blocking in a way: #74 .. it should ideally be resolved before proceeding with the above, but for now I can just pretend that there is only one html file that needs to be built.

[limonte]

without losing the <pre> element content from the static index.html

I guess you want to do that for SEO purposes?

[zenflow]

I guess you want to do that for SEO purposes?

Yes, exactly.

This will require some kind of a build process for index.html.

Not sure your feelings on that.. ?

My least invasive idea for this is to [in the build process] read in ./src/index.html (copy of current ./index.html with the example code removed), insert the example code content using cheerio, and save to ./index.html. This will also give us the power to pre-render other things if and when necessary or desireable.

[zenflow]

But if you don't care about SEO for the example code, then we can skip the whole "build index.html" thing.

[limonte]

But if you don't care about SEO for the example code, then we can skip the whole "build index.html" thing.

I think there's not much SEO value in the code snippets. If you agree with that, let's skip the whole "build index.html" thing.

As I see the implementation there should be empty <pre> blocks which will be populated dynamically. @zenflow is that how you're going to proceed?

[zenflow]

As I see the implementation there should be empty <pre> blocks which will be populated dynamically. @zenflow is that how you're going to proceed?

Yup, exactly. Another detail of how I plan to implement this, is that each example will have an ID which will be used to:

1. Mark the <pre> elements to say which example it should contain
   • e.g. <pre class="code-sample" data-example-id="customPositioning"></pre>
2. Execute the example
   • e.g. <button aria-label="..." onclick="executeExample('customPositioning')">Try me!</button>

@limonte Let me know if that all sounds good, and I'll make a PR

[limonte]

Let me know if that all sounds good, and I'll make a PR

LGTM 👍