-
Notifications
You must be signed in to change notification settings - Fork 0
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
feature: file model, download links & file redirects #147
Conversation
Deploying head-start with Cloudflare Pages
|
@@ -1,40 +1,46 @@ | |||
--- | |||
import { getLocale, locales, t } from '@lib/i18n'; | |||
import { getLocale, getLocaleName, locales, t } from '@lib/i18n'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really part of this PR. But implementing the FileLink
I discovered the Intl.DisplayNames
API, which led me to use it here and improve the accessibility of the locale selector a bit while going at it.
'nl': 'Nederlands', | ||
}; | ||
const localeName = localeNamesByCode[code as keyof typeof localeNamesByCode]; | ||
const localeName = new Intl.DisplayNames([code], { type: 'language' }).of(code); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really part of this PR. But implementing the FileLink
I discovered the Intl.DisplayNames
API, which is a much more robust and future-proof implementation of this helper function.
@@ -0,0 +1,9 @@ | |||
import type { FileLinkFragment } from '@lib/types/datocms'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm only using this in a single place right now, so could've just copied it there. In the No Dodos website I improved the routing and started a lib/routing/
setup which I hope we can reuse here. So this is a really small start for that :)
const redirects = fetchRedirectRules(); | ||
const existingFile = await readFile('./dist/_redirects', 'utf-8'); | ||
|
||
const combinedFile = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, this is disappointing. Apparently Cloudflare only supports pointing to relative URLs (so same-domain) 🤷 :
19:55:58.744 | Found invalid redirect lines:
-- | --
19:55:58.744 | - #2: /files/* https://www.datocms-assets.com/:splat 200
19:55:58.745 | Proxy (200) redirects can only point to relative paths. Got https://www.datocms-assets.com/:splat
19:55:58.745 | - #6: /my-files/example.pdf https://www.datocms-assets.com/104869/1718125906-dummy-with-custom-slug.pdf 200
19:55:58.745 | Proxy (200) redirects can only point to relative paths. Got https://www.datocms-assets.com/104869/1718125906-dummy-with-custom-slug.pdf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ow, here it is :)
Proxying
Proxying will only support relative URLs on your site. You cannot proxy external domains.
# Changes Use localised language names in locale selector for enhanced accessiblity. "en" -> "English", "nl" -> "Nederlands". # Associated issue N/A (was part of PR #147 which is blocked) # How to test 1. Open preview link 2. Hover over locale selector items 3. Review the localised language names # Checklist - [x] I have performed a self-review of my own code - [x] I have made sure that my PR is easy to review (not too big, includes comments) - [x] I have made updated relevant documentation files (in project README, docs/, etc) - ~~I have added a decision log entry if the change affects the architecture or changes a significant technology~~ - ~~I have notified a reviewer~~ <!-- Please strike through and check off all items that do not apply (rather than removing them) -->
todo
Changes
Associated issue
Resolves #148
Resolves #155
How to test
Checklist