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

devenv: inject <noscript-tag> after build #42

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

Conversation

vhsconnect
Copy link

Informs users that they need to whitelist the site to view the SPA. Couldn't find a way to do within elm.

closes #27

@@ -122,6 +123,10 @@ in
sed -i "s#Url.Builder.crossOrigin req.basePath req.pathParams#Url.Builder.absolute (\"api\" :: req.pathParams)#g" \
${config.devenv.root}/frontend/generated-api/src/Api.elm
'';
scripts.inject-noscript-tag.exec = ''
sed -i "s|.*placeholder-no-script-tag.*|<noscript><div>Cannot use Flaskestry.dev without javascript</div></noscript>|" \
Copy link

Choose a reason for hiding this comment

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

Can you give a better description than this? If you put yourself in the shoes of of someone who happened upon this site with a script blocker, your questions might be:

  • What is Flakestry?
  • Why does it require JavaScript (note the camelCasing) & what benefit will I get from allowing remote execution?

These questions should be minimally answered in the context of SPA where this is the only thing someone would see (no surrounding text, no link to an “About” page, etc.). Just telling someone to enable scripts while not giving the bare minimum for what & why is not a good user experience & will lead to page bounces.

… Also since this is a SPA, basic web crawlers & search engines will see only this text so why not give them something to actually index & show in their results?

In the linked issue this is supposed to close, I gave a sample of what might be better suited.

Copy link
Member

Choose a reason for hiding this comment

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

search engines / crawlers all support javascript these days.

Copy link

Choose a reason for hiding this comment

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

Not true: https://search.brave.com/search?q=flakestry.dev

No description or information about it since JS isn’t executed.

@vhsconnect
Copy link
Author

vhsconnect commented Feb 5, 2024

This works for me personally. I just want a reminder to whitelist the site instead of landing on a black screen. Also lets wait and see if @domenkozar will accept the pr.

Keep in mind that this is a tag that is injected into markup after the build. Adding link tags in that context is not so clean. Not sure how a link to an 'about' page would help? It's all spa - the user wouldn't be able to see anything there until he whitelists the site

@domenkozar
Copy link
Member

The correct way to do this is to define a global layout in Elm and put it there.

I'll hack on this during thaigersprint.org in two weeks.

@vhsconnect
Copy link
Author

Ok then, I'll keep this open but feel free to close it once you have the global layout in.
Have fun - looks amazing!

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.

Add noscript
3 participants