add outdated-version.html #844

sadiqkhoja · 2024-12-10T23:39:10Z

Related to #774

What has been done to verify that this works as intended?

Why is this the best possible solution? Were any other approaches considered?

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

Does this change require updates to documentation? If so, please file an issue here and include the link below.

Before submitting this PR, please make sure you have:

branched off and targeted the next branch OR only changed documentation/infrastructure (master is stable and used in production)
verified that any code or assets from external sources are properly credited in comments or that everything is internally sourced

matthew-white

This approach looks good to me!

matthew-white · 2024-12-11T18:43:08Z

docs/outdated-version.html

+        <span class="sentence1">Upgrade now to protect your data and take advantage of the latest features.</span>
+        <span class="sentence2">If you don't want to maintain Central, try<a href="https://getodk.org/#success-stories" target="_blank"> ODK Cloud.</a></span>
+      </p>
+      <script defer src='https://static.cloudflareinsights.com/beacon.min.js' data-cf-beacon='{"token": "81f288331d6e4638be205e0e63388165"}'></script>


Do we need this here? I feel like we just need it in news.html.

I think this will help us in version analytics

Ah I see. Will the analytics show the version query parameter?

I think cloudflareinsights logs full URL, let's confirm it with Yaw

matthew-white · 2024-12-11T18:46:39Z

docs/i18n.json

One thing we could do is create a component in Frontend with the source text, but never use the component in Frontend. The source text would still get sent to Transifex for translation though. We could copy the translations here whenever they change.

If we took that approach, the structure of the JSON would be [language] > [message key], not [message key] > [language].

I like that approach

matthew-white · 2024-12-11T18:49:37Z

docs/outdated-version.html

+          const localText = Object.keys(i18n).reduce((a, k) => {a.set(k, i18n[k][lang] ?? i18n[k]['en']); return a;}, new Map());
+          document.querySelector('.heading').textContent = localText.get('heading');
+          document.querySelector('.sentence1').textContent = localText.get('sentence1');
+          document.querySelector('.sentence2').innerHTML = localText.get('sentence2').replace('{{OdkCloudLink}}', odkCloudLink);


We don't need {{OdkCloudLink}} anymore, right? I feel a little unsure about setting innerHTML like this. I'd think about a translator adding malicious HTML to their translation of sentence2.

I think we need it. What do you think of making it safer:

const parts = localText.get('sentence2').split('{{OdkCloudLink}}'); document.querySelector('.sentence2').textContent = ''; document.querySelector('.sentence2').appendChild(document.createTextNode(parts[0])); document.querySelector('.sentence2').appendChild(odkCloudLink); document.querySelector('.sentence2').appendChild(document.createTextNode(parts[1]));

Sounds good. 👍

matthew-white · 2024-12-11T18:52:32Z

docs/outdated-version.html

+        const lang = queryParams.get('lang');
+        const odkCloudLink = '<a href="https://getodk.org/#success-stories" target="_blank">ODK Cloud</a>';
+
+        fetch('./i18n.json').then(r => r.json()).then(i18n => {


Rather than fetching it, I wouldn't have a problem with bundling all the translations in this file. There aren't many messages to translate, so I don't think it'd be too large. Either way (fetching or not) seems good to me.

matthew-white · 2024-12-11T18:53:25Z

docs/outdated-version.html

+
+          document.querySelector('.outdated-version').style = 'display: flex;'
+        })
+      })


Suggested change

})

});

matthew-white · 2024-12-11T18:55:24Z

docs/outdated-version.html

+          document.querySelector('.sentence1').textContent = localText.get('sentence1');
+          document.querySelector('.sentence2').innerHTML = localText.get('sentence2').replace('{{OdkCloudLink}}', odkCloudLink);
+
+          document.querySelector('.outdated-version').style = 'display: flex;'


I was confused at first as to why this was necessary. I thought it was just duplicating style.css. But it's needed because the element is initially hidden via display: none. How about adding a comment along those lines?

matthew-white · 2024-12-11T19:53:52Z

I changed the base branch to next, but maybe master would be fine in this case actually. We sort of need it deployed right away so that it can be shown in Frontend.

matthew-white · 2024-12-11T20:55:43Z

docs/outdated-version.html

+      "en": {
+        "heading": "You're using a significantly outdated version of ODK Central",
+        "sentence1": "Upgrade now to protect your data and take advantage of the latest features.",
+        "sentence2": "If you don't want to maintain Central, try {{OdkCloudLink}}"
+      },


If we have English in the DOM by default, could we remove it here? Or alternatively (maybe even better), could we remove it from the DOM and fill it in using these messages? The <h1> and <span>s in the DOM could start out as empty.

matthew-white · 2024-12-11T20:56:49Z

docs/outdated-version.html

+  <script>
+    const i18n = {
+      "en": {
+        "heading": "You're using a significantly outdated version of ODK Central",


I think curly quotes are a nice touch. 🤓

Suggested change

"heading": "You're using a significantly outdated version of ODK Central",

"heading": "You’re using a significantly outdated version of ODK Central",

matthew-white · 2024-12-11T20:59:47Z

docs/outdated-version.html

+      <span class="sentence2">If you don't want to maintain Central, try<a href="https://getodk.org/#success-stories"
+          target="_blank"> ODK Cloud.</a></span>


This is ever so slightly different from the English message below:

Here, the space between "try" and "ODK" is within the <a>, whereas below, it's before the <a>.

There's a period at the end of the sentence here, but not below. I think the period is nice.

matthew-white · 2024-12-12T06:52:54Z

We sort of need it deployed right away so that it can be shown in Frontend.

Just to be explicit, it will only be deployed right away if we merge the PR into the master branch, not next. The target branch is currently next, since I changed it.

sadiqkhoja force-pushed the master branch from 745a409 to d6c8f37 Compare December 10, 2024 23:40

matthew-white reviewed Dec 11, 2024

View reviewed changes

matthew-white changed the base branch from master to next December 11, 2024 18:59

sadiqkhoja force-pushed the master branch from d6c8f37 to 112de46 Compare December 11, 2024 20:40

matthew-white approved these changes Dec 11, 2024

View reviewed changes

sadiqkhoja force-pushed the master branch from 112de46 to b6491e5 Compare December 12, 2024 02:54

add outdated-version.html

a9acd8a

sadiqkhoja force-pushed the master branch from b6491e5 to a9acd8a Compare December 12, 2024 02:59

sadiqkhoja marked this pull request as ready for review December 12, 2024 17:01

sadiqkhoja changed the base branch from next to master December 12, 2024 17:01

sadiqkhoja merged commit 9a1d296 into getodk:master Dec 12, 2024
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add outdated-version.html #844

add outdated-version.html #844

sadiqkhoja commented Dec 10, 2024 •

edited

Loading

matthew-white left a comment

matthew-white Dec 11, 2024

sadiqkhoja Dec 11, 2024

matthew-white Dec 11, 2024

sadiqkhoja Dec 11, 2024

matthew-white Dec 11, 2024

sadiqkhoja Dec 11, 2024

matthew-white Dec 11, 2024

sadiqkhoja Dec 11, 2024

matthew-white Dec 11, 2024

matthew-white Dec 11, 2024

matthew-white Dec 11, 2024

matthew-white Dec 11, 2024

matthew-white commented Dec 11, 2024

matthew-white Dec 11, 2024

matthew-white Dec 11, 2024

matthew-white Dec 11, 2024

matthew-white commented Dec 12, 2024

	"heading": "You're using a significantly outdated version of ODK Central",
	"heading": "You’re using a significantly outdated version of ODK Central",

		<span class="sentence2">If you don't want to maintain Central, try<a href="https://getodk.org/#success-stories"
		target="_blank"> ODK Cloud.</a></span>

add outdated-version.html #844

add outdated-version.html #844

Conversation

sadiqkhoja commented Dec 10, 2024 • edited Loading

What has been done to verify that this works as intended?

Why is this the best possible solution? Were any other approaches considered?

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

Does this change require updates to documentation? If so, please file an issue here and include the link below.

Before submitting this PR, please make sure you have:

matthew-white left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthew-white commented Dec 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthew-white commented Dec 12, 2024

sadiqkhoja commented Dec 10, 2024 •

edited

Loading