Issue 1242 url preview #1247
Issue 1242 url preview #1247
Conversation
|
Deployed commit |
fflorent
force-pushed
the
issue-1242-url-preview
branch
from
64685b3 to
aeb4a24
Compare
|
Deployed commit |
fflorent
force-pushed
the
issue-1242-url-preview
branch
from
aeb4a24 to
715b5eb
Compare
|
Deployed commit |
fflorent
force-pushed
the
issue-1242-url-preview
branch
from
715b5eb to
99283f7
Compare
|
Deployed commit |
fflorent
force-pushed
the
issue-1242-url-preview
branch
from
9a72269 to
3aabb0a
Compare
|
Deployed commit |
fflorent
force-pushed
the
issue-1242-url-preview
branch
from
3aabb0a to
725aeb9
Compare
|
Deployed commit |
fflorent
force-pushed
the
issue-1242-url-preview
branch
from
725aeb9 to
b81bea2
Compare
|
Deployed commit |
app/server/lib/sendAppPage.ts
Outdated
| @@ -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 -->", getPageTitle(config) ?? (translate(req, 'Loading') + "...")) | |||
There was a problem hiding this comment.
I moved this line of code. I am a bit worried, because this seems to me a bad practice to concatenate a translated string with a punctuation.
I would rather move the ellipsis in the translation. Does it make sense to you?
There was a problem hiding this comment.
Yes at least for right to left languages.
|
Deployed commit |
|
Deployed commit |
|
Deployed commit |
|
Deployed commit |
|
Deployed commit |
static/img/icon-grist.png
Outdated
There was a problem hiding this comment.
May be use https://github.com/gristlabs/grist-core/blob/main/static/icons/grist.svg instead.
|
Hey, great work :) Maybe I'm saying this too late but… how about using a more "marketing" approach for og title/images? :) 1. About the image: Most social platforms recommend using a 1200x630px image as og:image, that is generally not just a logo, but something that looks more friendly when sharing the webpage. If you test this with the LinkedIn inspector for example, you see they target a different aspect ratio, as X does, as Facebook does, etc.:
Here is quickly made-up example of what we could do instead (not large enough in terms of pixels but with the correct aspect ratio):
2. About the title: I'd argue "Welcome - Grist" is also not that friendly as the main title for sharing purposes and would say having the tagline in it might be better. When showing the website preview card, some platforms show the description, some do not. So I'd say having Sidenote: when testing this with this tool: https://www.bannerbear.com/tools/twitter-card-preview-tool, |
|
@manuhabitela Thanks for your feedback. As discussed, I take a look of what I can, but I feel like it should not block the review as it is better than what already exist. Still this should be a nice follow-up.
I don't know how this tool works. The only thing I notice is that the preview is not rendered on Twitter, at the contrary of say https://duckduckgo.com I am trying something to fix this. |
|
You're totally right, my mistake for reporting that late :) |
|
Deployed commit |
|
Deployed commit |
fflorent
force-pushed
the
issue-1242-url-preview
branch
from
332f060 to
0e1727d
Compare
|
Deployed commit |
fflorent
force-pushed
the
issue-1242-url-preview
branch
from
ece1607 to
c6cb821
Compare
…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]>
fflorent
force-pushed
the
issue-1242-url-preview
branch
from
c6cb821 to
fdc7a36
Compare
|
Deployed commit |
|
@berhalak landing is blocked for the changes you requested, are you satisfied? |
paulfitz
dismissed
berhalak’s stale review
Jarek is pretty sick at the moment, so lightening his plate. Looked at the review and everything seems addressed. Dismissing the review since it is blocking landing and I want to take the task of dealing with this PR off Jarek's plate..
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
statics/img/opengraph-preview-image.png(introduced in this PR)Grist, the evolution of spreadsheet, and to[doc name] - Gristif 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 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?
Screenshots / Screencasts
Here is how the preview of the instance link looks like on Signal:
And how it looks like when previewing a document (:warning: Important: it must be shared publicly to obtain this result):
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