Skip to content

Commit

Permalink
Add description and icon to preview URLs for non-document resources g…
Browse files Browse the repository at this point in the history
…ristlabs#1242

When pasting a URL in some app or website that allows previewing the
link, for other resources than documents, you were offered an irrelevant
description. This patches aims to give a generic description of what is
Grist.

Co-authored-by: Emmanuel Pelletier <[email protected]>
  • Loading branch information
fflorent and manuhabitela committed Nov 18, 2024
1 parent 2735285 commit 656f6d8
Show file tree
Hide file tree
Showing 7 changed files with 67 additions and 32 deletions.
2 changes: 1 addition & 1 deletion app/server/lib/requestUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export function adaptServerUrl(url: URL, req: RequestWithOrg): void {
const reqBaseDomain = parseSubdomain(req.hostname).base;

if (process.env.GRIST_SERVE_SAME_ORIGIN === 'true' || req.isCustomHost) {
url.hostname = req.hostname;
url.host = req.headers.host || req.hostname;
} else if (reqBaseDomain) {
const subdomain: string|undefined = parseSubdomain(url.hostname).org || DEFAULT_HOME_SUBDOMAIN;
url.hostname = `${subdomain}${reqBaseDomain}`;
Expand Down
57 changes: 29 additions & 28 deletions app/server/lib/sendAppPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import jsesc from 'jsesc';
import * as path from 'path';
import difference = require('lodash/difference');

const translate = (req: express.Request, key: string, args?: any) => req.t(`sendAppPage.${key}`, args);
const translate = (req: express.Request, key: string, args?: any) => String(req.t(`sendAppPage.${key}`, args));

export interface ISendAppPageOptions {
path: string; // Ignored if .content is present (set to "" for clarity).
Expand Down Expand Up @@ -158,9 +158,9 @@ export function makeSendAppPage({ server, staticDir, tag, testLogin, baseDomain
const tagManagerSnippet = needTagManager ? getTagManagerSnippet(process.env.GOOGLE_TAG_MANAGER_ID) : '';
const staticTag = options.tag || tag;
// If boot tag is used, serve assets locally, otherwise respect
// APP_STATIC_URL.
const staticOrigin = staticTag === 'boot' ? '' : (process.env.APP_STATIC_URL || '');
const staticBaseUrl = `${staticOrigin}/v/${staticTag}/`;
// APP_STATIC_URL or APP_HOME_URL.
const staticOrigin = staticTag === 'boot' ? '' : (process.env.APP_STATIC_URL || config.homeUrl || '');
const staticBaseUrl = `${staticOrigin.replace(/\/*$/, '')}/v/${staticTag}/`;
const customHeadHtmlSnippet = server.create.getExtraHeadHtml?.() ?? "";
const warning = testLogin ? "<div class=\"dev_warning\">Authentication is not enforced</div>" : "";
// Preload all languages that will be used or are requested by client.
Expand All @@ -174,8 +174,8 @@ export function makeSendAppPage({ server, staticDir, tag, testLogin, baseDomain
).join('\n');
const content = fileContent
.replace("<!-- INSERT WARNING -->", warning)
.replace("<!-- INSERT TITLE -->", getPageTitle(req, config))
.replace("<!-- INSERT META -->", getPageMetadataHtmlSnippet(config))
.replace("<!-- INSERT TITLE -->", getDocName(config) ?? translate(req, 'Loading...'))
.replace("<!-- INSERT META -->", getPageMetadataHtmlSnippet(req, config, staticBaseUrl))
.replace("<!-- INSERT TITLE SUFFIX -->", getPageTitleSuffix(server.getGristConfig()))
.replace("<!-- INSERT BASE -->", `<base href="${staticBaseUrl}">` + tagManagerSnippet)
.replace("<!-- INSERT LOCALE -->", preloads)
Expand Down Expand Up @@ -264,18 +264,15 @@ function configuredPageTitleSuffix() {
}

/**
* Returns a page title suitable for inserting into an HTML title element.
*
* Currently returns the document name if the page being requested is for a document, or
* a placeholder, "Loading...", that's updated in the client once the page has loaded.
* Returns the doc name.
*
* Note: The string returned is escaped and safe to insert into HTML.
*
*/
function getPageTitle(req: express.Request, config: GristLoadConfig): string {
function getDocName(config: GristLoadConfig): string|null {
const maybeDoc = getDocFromConfig(config);
if (!maybeDoc) { return translate(req, 'Loading') + "..."; }

return handlebars.Utils.escapeExpression(maybeDoc.name);
return maybeDoc && handlebars.Utils.escapeExpression(maybeDoc.name);
}

/**
Expand All @@ -286,25 +283,29 @@ function getPageTitle(req: express.Request, config: GristLoadConfig): string {
*
* Note: The string returned is escaped and safe to insert into HTML.
*/
function getPageMetadataHtmlSnippet(config: GristLoadConfig): string {
function getPageMetadataHtmlSnippet(req: express.Request, config: GristLoadConfig, staticBaseUrl: string): string {
const metadataElements: string[] = [];
const maybeDoc = getDocFromConfig(config);

const description = maybeDoc?.options?.description;
if (description) {
const content = handlebars.Utils.escapeExpression(description);
metadataElements.push(`<meta name="description" content="${content}">`);
metadataElements.push(`<meta property="og:description" content="${content}">`);
metadataElements.push(`<meta name="twitter:description" content="${content}">`);
}
metadataElements.push('<meta property="og:type" content="website">');
metadataElements.push('<meta name="twitter:card" content="summary_large_image">');
const description = maybeDoc?.options?.description ?? translate(req, 'og-description');
const escapedDescription = handlebars.Utils.escapeExpression(description);
metadataElements.push(`<meta name="description" content="${escapedDescription}">`);
metadataElements.push(`<meta property="og:description" content="${escapedDescription}">`);
metadataElements.push(`<meta name="twitter:description" content="${escapedDescription}">`);

const icon = maybeDoc?.options?.icon;
if (icon) {
const content = handlebars.Utils.escapeExpression(icon);
metadataElements.push(`<meta name="thumbnail" content="${content}">`);
metadataElements.push(`<meta property="og:image" content="${content}">`);
metadataElements.push(`<meta name="twitter:image" content="${content}">`);
}
const icon = maybeDoc?.options?.icon ?? new URL('img/opengraph-preview-image.png', staticBaseUrl).href;
const escapedIcon = handlebars.Utils.escapeExpression(icon);
metadataElements.push(`<meta name="thumbnail" content="${escapedIcon}">`);
metadataElements.push(`<meta property="og:image" content="${escapedIcon}">`);
metadataElements.push(`<meta name="twitter:image" content="${escapedIcon}">`);

const maybeDocTitle = getDocName(config);
const title = (maybeDocTitle ? maybeDocTitle + getPageTitleSuffix(config) : translate(req, 'og-title'));
// NB: We don't generate the content of the <title> tag here.
metadataElements.push(`<meta property="og:title" content="${title}">`);
metadataElements.push(`<meta name="twitter:title" content="${title}">`);

return metadataElements.join('\n');
}
Expand Down
Binary file added static/img/opengraph-preview-image.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 3 additions & 1 deletion static/locales/en.server.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
{
"sendAppPage": {
"Loading": "Loading"
"Loading...": "Loading...",
"og-description": "A modern, open source spreadsheet that goes beyond the grid",
"og-title": "Grist, the evolution of spreadsheets"
},
"oidc": {
"emailNotVerifiedError": "Please verify your email with the identity provider, and log in again."
Expand Down
2 changes: 1 addition & 1 deletion static/locales/fr.server.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"sendAppPage": {
"Loading": "Chargement"
"Loading...": "Chargement..."
}
}
17 changes: 17 additions & 0 deletions test/nbrowser/HomeIntro.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ describe('HomeIntro', function() {
});

it('should should intro screen for anon', () => testIntroScreen({anon: true, team: false}));
it('should should set meta tags for URL previews', testMetaTags);
it('should not show Other Sites section', testOtherSitesSection);
it('should allow create/import from intro screen', testCreateImport.bind(null, false));
it('should link to examples page from the intro', testExamplesPage);
Expand Down Expand Up @@ -129,6 +130,22 @@ describe('HomeIntro', function() {
}
}

async function testMetaTags() {
const expectedTitle = 'Grist, the evolution of spreadsheets';
assert.equal(await driver.find('meta[name="twitter:title"]').getAttribute('content'), expectedTitle);
assert.equal(await driver.find('meta[property="og:title"]').getAttribute('content'), expectedTitle);

const expectedDescription = 'A modern, open source spreadsheet that goes beyond the grid';
assert.equal(await driver.find('meta[name="description"]').getAttribute('content'), expectedDescription);
assert.equal(await driver.find('meta[name="twitter:description"]').getAttribute('content'), expectedDescription);
assert.equal(await driver.find('meta[property="og:description"]').getAttribute('content'), expectedDescription);

const gristIconFileName = 'opengraph-preview-image.png';
assert.include(await driver.find('meta[name="thumbnail"]').getAttribute('content'), gristIconFileName);
assert.include(await driver.find('meta[name="twitter:image"]').getAttribute('content'), gristIconFileName);
assert.include(await driver.find('meta[property="og:image"]').getAttribute('content'), gristIconFileName);
}

async function testCreateImport(isLoggedIn: boolean) {
await checkIntroButtons(isLoggedIn);

Expand Down
17 changes: 16 additions & 1 deletion test/nbrowser/NewDocument.ntest.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,22 @@ describe('NewDocument.ntest', function() {
this.timeout(10000);
await gu.actions.createNewDoc('Untitled');
assert.equal(await gu.actions.getDocTitle(), 'Untitled');
assert.equal(await driver.getTitle(), 'Untitled - Grist');

const expectedTitle = 'Untitled - Grist';
assert.equal(await driver.getTitle(), expectedTitle);
assert.equal(await driver.find('meta[name="twitter:title"]').getAttribute('content'), expectedTitle);
assert.equal(await driver.find('meta[property="og:title"]').getAttribute('content'), expectedTitle);

const expectedDescription = 'A modern, open source spreadsheet that goes beyond the grid';
assert.equal(await driver.find('meta[name="description"]').getAttribute('content'), expectedDescription);
assert.equal(await driver.find('meta[name="twitter:description"]').getAttribute('content'), expectedDescription);
assert.equal(await driver.find('meta[property="og:description"]').getAttribute('content'), expectedDescription);

const gristIconFileName = 'opengraph-preview-image.png';
assert.include(await driver.find('meta[name="thumbnail"]').getAttribute('content'), gristIconFileName);
assert.include(await driver.find('meta[name="twitter:image"]').getAttribute('content'), gristIconFileName);
assert.include(await driver.find('meta[property="og:image"]').getAttribute('content'), gristIconFileName);

assert.equal(await $('.active_section .test-viewsection-title').wait().text(), 'TABLE1');
await gu.waitForServer();
});
Expand Down

0 comments on commit 656f6d8

Please sign in to comment.