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.

Sign up for GitHub

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

Jump to bottom

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

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

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions devenv.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ let
generate-elm-api
pushd frontend
elm-land build
inject-noscript-tag
popd
devenv container ${env} --copy
flyctl deploy --vm-memory 1024 -a flakestry-${env} \
Expand Down Expand Up @@ -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.

${config.devenv.root}/frontend/dist/index.html
'';

processes = {
backend.exec = "cd ${config.devenv.root} && uvicorn --app-dir backend ${lib.optionalString (!config.container.isBuilding) "--reload"} flakestry.main:app";
Expand Down
3 changes: 2 additions & 1 deletion frontend/elm-land.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@
"link": [
{ "rel": "stylesheet", "href": "https://rsms.me/inter/inter.css" },
{ "rel": "stylesheet", "href": "https://cdnjs.cloudflare.com/ajax/libs/highlight.js/11.8.0/styles/github.min.css"},
{ "rel": "icon", "type": "image/x-icon", "href": "/logo.png" }
{ "rel": "icon", "type": "image/x-icon", "href": "/logo.png" },
{ "rel": "placeholder-no-script-tag", "href": "#" }
],
"script": []
},
Expand Down