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

Tweak some samples for clarity and better behavior in playgrounds #148

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

tigt
Copy link
Contributor

@tigt tigt commented Jan 23, 2023

Use viewport dimensions instead of document title

Since most framework playgrounds output into an <iframe>, their document.title properties end up being the empty string unless set by the component code. This made the samples look broken when loaded into playgrounds, so instead this uses runtime properties that every <iframe> can’t help but have.

Simplify traffic light logic

The lightIndex selection logic can be simplified, which seems like a net win for the goal of this site; less attention on the non-framework-specific parts means more attention on the differences between components.

Use mixed checkbox instead of .focus() for DOM refs

When opening the existing elementRef.focus() examples in their frameworks’ playgrounds, the call to .focus() would be ignored because of <iframe> reasons or something. This change no longer seems like it’s broken when attempting to view it, and as a bonus it gets rid of the ambiguity between which frameworks used type="text" and which omitted it (Was it because the framework needs explicit attributes with default values, or because some example authors preferred to leave it off?)

When opening the existing `elementRef.focus()` examples in their frameworks’ playgrounds,
the call to `.focus()` would be ignored because of `<iframe>` reasons or something. This
change no longer seems like it’s broken when attempting to view it, and as a bonus it
gets rid of the ambiguity between which frameworks used `type="text"` and which omitted
it (Was it because the framework needs explicit attributes with default values, or
because some example authors preferred to leave it off?)
The `lightIndex` selection logic can be simplified, which seems like a net win for the
goal of this site; less attention on the non-framework-specific parts means more
attention on the differences between components.
Since most framework playgrounds output into an `<iframe>`, their
`document.title` properties end up being the empty string unless
set by the component code. This made the samples look broken when
loaded into playgrounds, so instead this uses runtime properties
that every `<iframe>` can’t help but have.
@tigt
Copy link
Contributor Author

tigt commented Jan 24, 2023

These are some things that bothered me enough to try improving them while working on adding Marko to the framework comparison. If you’re down with the changes, I can quickly follow this up with that, but if you’d rather not, I’ll update the eventual Marko PR to stay consistent.

This was referenced Jan 30, 2023
@matschik
Copy link
Owner

matschik commented Feb 9, 2023

I chose document.title and .focus methods for the following reasons:

  • well known to web developers
  • used in real world apps
  • simple to use

It bothers me that it does not work well in playgrounds... Sacrificing readability and therefore comprehension to work inside an iframe is not worth it... Not sure which well know apis to use to make it work in playgrounds and CP snippet requirements.
viewport and indeterminate apis are not well known, that's a hard dilemma.
I'll make some search to find a possible other solution.

@matschik
Copy link
Owner

matschik commented Feb 9, 2023

About on mount example, I was thinking about this:

<script>
  import { onMount } from "svelte";
	
  onMount(() => {
    console.log(document.getElementById("website").value)
  });
</script>

<input id="website" value="Component Party" />

But it's not a recommended way to use the DOM, and it's mixing with dom ref examples.

@tigt
Copy link
Contributor Author

tigt commented Feb 17, 2023

I agree with your point about indeterminate, but are viewport dimensions that obscure? Finding the dimensions of a web page in JS seems like a pretty reasonable question for newbies, and even if they’re unfamiliar with the concept, every page having a width and height doesn’t seem onerous to ask them to imagine.

For the DOM ref example purposes, one way to go would be a property/method that interacts with the element that isn’t settable as an HTML attribute. Some possibilities:

  • playbackSpeed on <audio> or <video>
  • showModal() on <dialog>
  • setCustomValidity() on form elements
  • select() on form elements (this one might break in playgrounds too, haven’t checked)
  • <canvas> in general

@matschik
Copy link
Owner

matschik commented Feb 21, 2023

Like you, I would be very happy that Marko is featured in CP.
Just to move forward, I got 2 proposals:

  • Totally disable playground feature for Marko (easy)
  • Disable playground for examples that don't work well in the playground (harder but cleaner)

What do you think ?

@tigt
Copy link
Contributor Author

tigt commented Mar 5, 2023

I’m happy to change the Marko code samples to match the others to get it in. (Sorry about how long this is taking — I got laid off by eBay shortly after I opened these PRs.)

To be clear, the issues with focus and document.title affect most of the playgrounds, not just Marko’s — it seemed like a confusing experience to show demo code that then appears to not work at all when taken for a test drive.

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

Successfully merging this pull request may close these issues.

None yet

2 participants