Skip to content

Commit

Permalink
Issue 1242 url preview (#1247)
Browse files Browse the repository at this point in the history
## Context

When previewing the URL of an instance (say https://docs.getgrist.com/ )
in websites or apps, it simply displays "Loading..." without any further
description or images. It's quite confusing.

See #1242 for detailed steps to reproduce.

## Proposed solution

- the description defaults to "Grist is the evolution of spreadsheets."
and is translatable
- the icon defaults to `statics/img/opengraph-preview-image.png`
(introduced in this PR)
- the title in meta descriptions defaults to `Grist, the evolution of
spreadsheet`, and to `[doc name] - Grist` if a document is consulted.

**Also a technical note**: I had to change the way requestUtils deduce
the host of the home, so it takes into account the port. Otherwise [this
test
failed](https://github.com/gristlabs/grist-core/blob/e9b5b98bcb35e1c9ae9e82eac6e7e8886891cfb4/test/nbrowser/AdminPanel.ts#L337-L343)
with my change, as I now specify the absolute URL for serving static
resources (needed for specifying the opengraph icon).

## Related issues

Fixes #1242 

## Has this been tested?

<!-- Put an `x` in the box that applies: -->

- [x] 👍 yes, I added tests to the test suite
- [ ] 💭 no, because this PR is a draft and still needs work
- [ ] 🙅 no, because this is not relevant here
- [ ] 🙋 no, because I need help <!-- Detail how we can help you -->

## Screenshots / Screencasts

Here is how the preview of the instance link looks like on Signal: 
![preview home
page](https://github.com/user-attachments/assets/d2600441-e120-4e76-975e-23b4a07598ec)


And how it looks like when previewing a document (:warning:
**Important**: it must be shared publicly to obtain this result):
![preview doc
title](https://github.com/user-attachments/assets/6ddd7f5e-6f44-4ba6-87c9-55a9d660696c)


You also may check this website which seems to show the rendering of URL
previews on different websites:
https://opengraph.dev/

Home page:
https://opengraph.dev/panel?url=https%3A%2F%2Fgrist-fflorent-grist-core-issue-1242-url-preview.fly.dev%2F
Doc page title:
https://opengraph.dev/panel?url=https%3A%2F%2Fgrist-fflorent-grist-core-issue-1242-url-preview.fly.dev%2FnbutBLRC2wSD%2Fpublic-doc

---------

Co-authored-by: Emmanuel Pelletier <[email protected]>
  • Loading branch information
fflorent and manuhabitela authored Jan 16, 2025
1 parent 50d8713 commit d06def9
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 28 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,7 @@ Grist can be configured in many ways. Here are the main environment variables it
| GRIST_CONTACT_SUPPORT_URL | set the link to contact support on error pages (example: email adress or online form) |
| GRIST_SUPPORT_ANON | if set to 'true', show UI for anonymous access (not shown by default) |
| GRIST_SUPPORT_EMAIL | if set, give a user with the specified email support powers. The main extra power is the ability to share sites, workspaces, and docs with all users in a listed way. |
| GRIST_OPEN_GRAPH_PREVIEW_IMAGE | the URL of the preview image when sharing the link on websites like social medias or chat applications. |
| GRIST_TELEMETRY_LEVEL | the telemetry level. Can be set to: `off` (default), `limited`, or `full`. |
| GRIST_THROTTLE_CPU | if set, CPU throttling is enabled |
| GRIST_TRUST_PLUGINS | if set, plugins are expect to be served from the same host as the rest of the Grist app, rather than from a distinct host. Ordinarily, plugins are served from a distinct host so that the cookies used by the Grist app are not automatically available to them. Enable this only if you understand the security implications. |
Expand Down
1 change: 1 addition & 0 deletions app/common/gristUrls.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ export const commonUrls = {
formulaSheet: 'https://support.getgrist.com/formula-cheat-sheet',
formulas: 'https://support.getgrist.com/formulas',
forms: 'https://www.getgrist.com/forms/?utm_source=grist-forms&utm_medium=grist-forms&utm_campaign=forms-footer',
openGraphPreviewImage: 'https://grist-static.com/icons/opengraph-preview-image.png',

gristLabsCustomWidgets: 'https://gristlabs.github.io/grist-widget/',
gristLabsWidgetRepository: 'https://github.com/gristlabs/grist-widget/releases/download/latest/manifest.json',
Expand Down
66 changes: 40 additions & 26 deletions app/server/lib/sendAppPage.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {
commonUrls,
Features,
getContactSupportUrl,
getFreeCoachingCallUrl,
Expand All @@ -24,9 +25,19 @@ import * as fse from 'fs-extra';
import * as handlebars from 'handlebars';
import jsesc from 'jsesc';
import * as path from 'path';
import difference = require('lodash/difference');
import difference from 'lodash/difference';

const { escapeExpression } = handlebars.Utils;

/**
* Return the translation given the key.
*
* @param req
* @param key The key of the translation (which will be prefixed by `sendAppPage`)
* @param args The args to pass to the translation string (optional)
*/
const translate = (req: express.Request, key: string, args?: any) => req.t(`sendAppPage.${key}`, args)?.toString();

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

export interface ISendAppPageOptions {
path: string; // Ignored if .content is present (set to "" for clarity).
Expand Down Expand Up @@ -174,8 +185,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) ?? escapeExpression(translate(req, 'Loading...')))
.replace("<!-- INSERT META -->", getPageMetadataHtmlSnippet(req, config))
.replace("<!-- INSERT TITLE SUFFIX -->", getPageTitleSuffix(server.getGristConfig()))
.replace("<!-- INSERT BASE -->", `<base href="${staticBaseUrl}">` + tagManagerSnippet)
.replace("<!-- INSERT LOCALE -->", preloads)
Expand Down Expand Up @@ -264,18 +275,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 && escapeExpression(maybeDoc.name);
}

/**
Expand All @@ -286,25 +294,31 @@ 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): 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 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 description = maybeDoc?.options?.description ?
escapeExpression(maybeDoc.options.description) :
translate(req, 'og-description');
metadataElements.push(`<meta name="description" content="${description}">`);
metadataElements.push(`<meta property="og:description" content="${description}">`);
metadataElements.push(`<meta name="twitter:description" content="${description}">`);

const openGraphPreviewImage = process.env.GRIST_OPEN_GRAPH_PREVIEW_IMAGE || commonUrls.openGraphPreviewImage;
const image = escapeExpression(maybeDoc?.options?.icon ?? openGraphPreviewImage);
metadataElements.push(`<meta name="thumbnail" content="${image}">`);
metadataElements.push(`<meta property="og:image" content="${image}">`);
metadataElements.push(`<meta name="twitter:image" content="${image}">`);

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
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
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 d06def9

Please sign in to comment.