Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Have the measurement unit depend on the timezone (which is based on the metadata coordinates). #111

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
28 changes: 28 additions & 0 deletions edge-apps/weather/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,35 @@
</div>
</section>
</div>

<script>
const disableAnalytics = screenly.settings.disable_analytics == "true";

if (disableAnalytics != "true") {
const sentryId = screenly.settings.sentry_id;
const gaApiKey = screenly.settings.ga_api_key;

const sentryScript = document.createElement('script');
sentryScript.src = `https://js.sentry-cdn.com/${sentryId}.min.js`;
sentryScript.crossOrigin = 'anonymous';
document.head.appendChild(sentryScript);

const gtagScript = document.createElement('script');
gtagScript.src = `https://www.googletagmanager.com/gtag/js?id=${gaApiKey}`;
gtagScript.async = true;
document.head.appendChild(gtagScript);

window.dataLayer = window.dataLayer || [];
function gtag(){dataLayer.push(arguments);}

gtag('js', new Date());
gtag('config', gaApiKey);
}
</script>

<script src="static/js/tz.min.js"></script>
<script src="static/js/moment-with-locales.min.js"></script>
<script src="static/js/moment-timezone-with-data.min.js"></script>
<script src="static/js/icons.js"></script>
<script src="static/js/main.js"></script>
</body>
Expand Down
25 changes: 18 additions & 7 deletions edge-apps/weather/static/js/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,17 @@ function initApp (data) {
*/

const countriesUsingFahrenheit = ['US', 'BS', 'KY', 'LR', 'PW', 'FM', 'MH']
const celsiusToFahrenheit = (temp) => ((1.8 * temp) + 32)
const getTemp = (temp) => Math.round(tempScale === 'C' ? temp : celsiusToFahrenheit(temp))
const timeZonesUsingFahrenheit = countriesUsingFahrenheit.map(
country => {
let timezones = moment.tz.zonesForCountry(country)

if (country === "BS") {
timezones = timezones.filter(tz => tz !== 'America/Toronto')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, no. This is a slippery slope. Please find a library that can do this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a particular reason we want to tie time units to timezone instead of country?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a particular reason we want to tie time units to timezone instead of country?

how are timezone and countries correlated? A country might have a lot of timezones, no?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, according to ChatGPT at least, there is no country that uses both. Thus mapping it to country would be fine. Yet, I want a library for this rather than a hard coded list (which can get long i guess).

}

return timezones
}
).flat()

const checkIfNight = (dt) => {
const dateTime = moment.unix(dt).utcOffset(tz)
Expand Down Expand Up @@ -181,7 +190,7 @@ function initApp (data) {
const updateCurrentWeather = (icon, desc, temp) => {
updateAttribute('current-weather-icon', 'src', icons[icon])
updateContent('current-weather-status', desc)
updateContent('current-temp', getTemp(temp))
updateContent('current-temp', Math.round(temp))
updateContent('current-temp-scale', `\u00B0${tempScale}`)
}

Expand Down Expand Up @@ -237,7 +246,7 @@ function initApp (data) {
const dummyNode = document.querySelector('.dummy-node')
const node = dummyNode.cloneNode(true)
node.classList.remove('dummy-node')
node.querySelector('.item-temp').innerText = getTemp(temp)
node.querySelector('.item-temp').innerText = Math.round(temp)
node.querySelector('.item-icon').setAttribute('src', icons[icon])
node.querySelector('.item-time').innerText = index === 0 ? 'Current' : formatTime(dateTime)

Expand All @@ -253,7 +262,7 @@ function initApp (data) {
const updateData = () => {
getDataFromIndexedDB(db, (data) => {
const { name, country, timezone } = data
tempScale = countriesUsingFahrenheit.includes(country) ? 'F' : 'C'
tempScale = timeZonesUsingFahrenheit.includes(tzlookup(lat, lng)) ? 'F' : 'C'
updateLocation(name)
tz = setTimeZone(timezone)
initDateTime()
Expand Down Expand Up @@ -289,12 +298,14 @@ function initApp (data) {

const fetchWeather = async () => {
try {
const timezone = tzlookup(lat, lng)
const units = timeZonesUsingFahrenheit.includes(timezone) ? 'imperial' : 'metric'
const endpointUrl = `https://api.openweathermap.org/data/2.5/forecast`
const apiKey = screenly.settings.openweathermap_api_key
const queryParams = stringifyQueryParams({
lat: lat,
lon: lng,
units: 'metric', // TODO: Make this dependent on the current location.
units,
cnt: 10,
appid: apiKey,
})
Expand Down Expand Up @@ -323,4 +334,4 @@ function initApp (data) {
init()
}

initApp()
window.onload = initApp;

Large diffs are not rendered by default.

5 changes: 5 additions & 0 deletions edge-apps/weather/static/js/tz.min.js

Large diffs are not rendered by default.