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

Make initial frame content useful #12

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

dracos
Copy link

@dracos dracos commented Dec 23, 2020

If the JavaScript does not run for whatever reason, provide a link so that the set side emails can still be seen.

(I wondered also whether to include a reference to the before-visit event that could be used as a hook to show the spinner, but wasn't sure.)

If the JavaScript does not run for whatever reason, provide a link so that the set side emails can still be seen.
@netlify
Copy link

netlify bot commented Dec 23, 2020

Deploy preview for gifted-meitner-072014 ready!

Built with commit 473b1df

https://deploy-preview-12--gifted-meitner-072014.netlify.app

@@ -74,11 +74,11 @@ In the example above, the trays start empty, but it's also possible to populate

```html
<turbo-frame id="set_aside_tray" src="/emails/set_aside">
<img src="/icons/spinner.gif">
<a href="/emails/set_aside">Set aside emails</a>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you feel about declaring both, with the Set aside link wrapped in a <noscript> element?

Copy link
Author

@dracos dracos Dec 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spinner could be confusing if it is shown and the JavaScript does not run for whatever reason, and the user waits and thinks something is happening when it is not, so I think it would be best if the spinner was only shown if the relevant JavaScript is running. Similarly, JavaScript might be enabled (so noscript would not be shown) but something goes wrong loading/running the JavaScript, and then the user has no way to proceed, so I don't think wrapping it in noscript is the best way to proceed.

But yes, the spinner could be present initially in the HTML if you had, I dunno e.g.

<img class="fetch-show" src="/icons/spinner.gif">
<a class="fetch-hide" href="/emails/set_aside">Set aside emails</a>

with some CSS like

.fetch-show { display: none; }

and then the JS that kicks off the fetch / some event handler at that point / whatever shows #set_aside_tray .fetch-show and hides #set_aside_tray .fetch-hide – and then if something goes wrong fetching the page it can revert back to showing the link easily enough, which would also be a good thing, more robust and resilient.
Hope that's helpful.

PS I realise this is only an early example in the documentation, but I really do think it's worthwhile to have something robust like this in such an example, because it might well be used as a template in many implementations. And going by the past N years of much web development I don't want people to start using the good ideas of this but still make something that is a bunch of non-working spinners :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants