make 'contact support' link customisable #854
Conversation
There was a problem hiding this comment.
Thanks @vviers. The new variable makes sense to me. Agree there's room to organize things, but feels ok to add one more for now.
README.md
Outdated
| @@ -263,7 +263,8 @@ GRIST_FORCE_LOGIN | Much like GRIST_ANON_PLAYGROUND but don't support anonymo | |||
| GRIST_SINGLE_ORG | set to an org "domain" to pin client to that org | |||
| GRIST_TEMPLATE_ORG | set to an org "domain" to show public docs from that org | |||
| GRIST_HELP_CENTER | set the help center link ref | |||
| FREE_COACHING_CALL_URL | set the link to the human help (example: email or meeting scheduling tool) | |||
| FREE_COACHING_CALL_URL | set the link to the human help (example: email adress or meeting scheduling tool) | |||
| CONTACT_SUPPORT_URL | set the link to contact support on error pages (example: email adress or online form) | |||
There was a problem hiding this comment.
| CONTACT_SUPPORT_URL | set the link to contact support on error pages (example: email adress or online form) | |
| GRIST_CONTACT_SUPPORT_URL | set the link to contact support on error pages (example: email adress or online form) |
Nitpicking here, but prefixing is a little bit safer (wish I remembered to suggest it for FREE_COACHING_CALL_URL before it was added).
app/common/gristUrls.ts
Outdated
| export function contactSupportUrl(): string|null { | ||
| if(isClient()) { | ||
| const gristConfig: GristLoadConfig = (window as any).gristConfig; | ||
| return gristConfig && gristConfig.contactSupportUrl || null; | ||
| } else { | ||
| return process.env.CONTACT_SUPPORT_URL || null; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
This and getFreeCoachingCallUrl are both unused, right? We've used functions like these in the past in gristUrls.ts to unify shared URLs in client and server code. That's probably not a bad idea here, as we reach for URLs from that file often, and we may not always remember to use the URLs from window.gristConfig.
It should be an easy refactor if you're up for it: updating commonUrls to call these functions for the contact and (new) freeCoachingCall keys, and updating errorPages.ts (and wherever the coaching call URL currently lives) to reference them. I'd also change the || null to || <fallback url>, and update makeGristConfig in sendAppPage.ts to replace the URLs like process.env.CONTACT_SUPPORT_URL || "https://www.getgrist.com/contact/" with calls to these functions as well. Wdyt?
app/client/ui/WelcomeCoachingCall.ts
Outdated
| @@ -103,7 +104,7 @@ We can show you the Grist basics, or start working with your data right away to | |||
| logTelemetryEvent('clickedScheduleCoachingCall'); | |||
| }), | |||
| { | |||
| href: getGristConfig().freeCoachingCallUrl, | |||
| href: getFreeCoachingCallUrl(), | |||
There was a problem hiding this comment.
| href: getFreeCoachingCallUrl(), | |
| href: commonUrls.freeCoachingCall, |
You can add this to app/common/gristUrls.ts:
export const commonUrls = {
freeCoachingCall: getFreeCoachingCallUrl(),
...,
};
app/client/ui/errorPages.ts
Outdated
| @@ -105,7 +105,7 @@ export function createNotFoundPage(appModel: AppModel, message?: string) { | |||
| })), | |||
| cssButtonWrap(bigPrimaryButtonLink(t("Go to main page"), testId('error-primary-btn'), | |||
| urlState().setLinkUrl({}))), | |||
| cssButtonWrap(bigBasicButtonLink(t("Contact support"), {href: getGristConfig().contactSupportUrl})), | |||
| cssButtonWrap(bigBasicButtonLink(t("Contact support"), {href: getContactSupportUrl()})), | |||
There was a problem hiding this comment.
| cssButtonWrap(bigBasicButtonLink(t("Contact support"), {href: getContactSupportUrl()})), | |
| cssButtonWrap(bigBasicButtonLink(t("Contact support"), {href: commonUrls.contact})), |
You can do the same here and just update the value of that key to be getContactSupportUrl().
Making sure you don't have our users filling support tickets with you @anaisconce ;)
I thought about reusing an existing environment variable because it would make sense in our case (as of right now, we redirect most "get help" / "coaching" / "contact us" URLs to our email address) but decided against it for now. I still think there is room for factorization in environment variables though.
wdyt @paulfitz @georgegevoian ?