OKRS-24-55 #1243

dmytrotsko · 2024-02-26T18:21:01Z

Prerequisites:

Branch is up-to-date with the branch to be merged with, i.e. dev
Build is successful
Code is cleaned up and formatted

Summary

"latest known data" link now preserves context.
Set default date to the date when we have data for all 3 sensors in 'Overview'. When date is present in URL, the date doesn't change to "default date". Added "NoRecentDataWarning" in order to nofity user about "old" data

netlify · 2024-02-26T18:22:35Z

✅ Preview link ready!

Name	Link
🔨 Latest commit	`a465b1a`
🔍 Latest deploy log	https://app.netlify.com/sites/cmu-delphi-covidcast/deploys/6627d894b13ec90008b3e315
😎 Deploy Preview	https://deploy-preview-1243--cmu-delphi-covidcast.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

…oved annotation for Hosptal Admissions sensor when we don't have recent data for choosen date

melange396

This looks great! I especially appreciate how you got rid of the asterisks and added tooltip hovers for the latest dates of each of the 3 highlighted indicators.

There is one behavior that we might want to change though (and we can merge this PR and fix it later, if necessary) -- when you go to the dashboard without any url parameters (like https://deploy-preview-1243--cmu-delphi-covidcast.netlify.app/ ), it shows the warning message as appropriate. However, when you go to the dashboard with a date specified that matches minMaxDate, it still shows the warning message (for instance, as of today 20240416, the most recent date with all 3 indicators available is 20240329, so today this happens when you visit https://deploy-preview-1243--cmu-delphi-covidcast.netlify.app/?date=20240329 ). This doesn't give incorrect information, but it could be fairly confusing for users.

melange396 · 2024-04-15T22:57:04Z

src/blocks/IndicatorWarning.svelte

-        <a href="?date={formatAPITime(sensor.timeFrame.max)}" on:click={switchDate}
-          >{formatDateYearDayOfWeekAbbr(sensor.timeFrame.max)}</a
+        <a
+          href="?date={formatAPITime(sensor.timeFrame.max)}&{window.location.search.split('&').slice(1).join('&')}"


Is this file still used anywhere? It looks like it has been replaced by your new NoRecentDataWarning.svelte...

If we are keeping the file, we should include a comment describing what this split/slice/join operation is doing.

I didn't mean to replace "IndicatorWarning.svelte" by "NoRecentDataWarning.svelte", but I didn't want to change existing "Indicator.svelte" either.
I would prefer to add comment and leave both of them (at least for now).

src/modes/summary/Overview.svelte

src/blocks/NoRecentDataWarning.svelte

src/modes/summary/Overview.svelte

src/blocks/NoRecentDataWarning.svelte

Co-authored-by: george <george.haff@gmail.com>

dmytrotsko · 2024-04-17T13:31:24Z

This looks great! I especially appreciate how you got rid of the asterisks and added tooltip hovers for the latest dates of each of the 3 highlighted indicators.

There is one behavior that we might want to change though (and we can merge this PR and fix it later, if necessary) -- when you go to the dashboard without any url parameters (like https://deploy-preview-1243--cmu-delphi-covidcast.netlify.app/ ), it shows the warning message as appropriate. However, when you go to the dashboard with a date specified that matches minMaxDate, it still shows the warning message (for instance, as of today 20240416, the most recent date with all 3 indicators available is 20240329, so today this happens when you visit https://deploy-preview-1243--cmu-delphi-covidcast.netlify.app/?date=20240329 ). This doesn't give incorrect information, but it could be fairly confusing for users.

Yeah, I agree that it is uncorrect behaviour and I wanted to fix that, but I have not found how to do that yet.

…comment to explain some split/slice logic in IndicatorWarning

…ting

…typo

…warning message handling.

…d debug logging

…explanation on split/slice/join operations in NoRecentDataWarning.svelte

melange396

This is great! Just two little things for punctuation, and then one comment/question about code location.

melange396 · 2024-04-23T15:25:08Z

src/modes/summary/Overview.svelte

+  // warningType is indicator of which exact warning message should be shown.
+  // By default, when user opens page with no specified date, the date will be set to the latest date we have data for all 3 indicators.
+  // In this case, warningType should be set to 1.
+  // In case selected date is set to future date (date > minMaxDate, where we don't have recent data for all 3 indicators), the warningType will be set to 2
+  // which has different warning message.
+  // In case selected date is set to some date which is < minMaxDate, the warningType will be set to 0 which means that we will not show
+  // any warning message.
+
+  // warningType should be set in beforeUpdate() method, to guess correct warningType.
+
+  let warningType = 1;
+  beforeUpdate(() => {
+    if (date.value > minMaxDate) {
+      warningType = 2;
+    } else if (date.value < minMaxDate) {
+      warningType = 0;
+    } else {
+      warningType = 1;
+    }
+  });


can we do the date.value != minMaxDate comparisons inside NoRecentDataWarning.svelte? That way we wouldn't have to pass warningType... Or do we need to do the comparisons inside of the beforeUpdate() event here?

i talked to Dmytro on slack, he is going to look at this but we will release the current state in the meantime as the functionality is doing what its supposed to.

src/blocks/NoRecentDataWarning.svelte

Co-authored-by: george <george.haff@gmail.com>

melange396

awesome!

melange396 · 2024-04-23T15:45:58Z

ah crap, build is now failing because prettier doesnt like something in src/blocks/NoRecentDataWarning.svelte

…ting

melange396 · 2024-04-23T15:53:36Z

thanks Dmytro, youre the best!

dmytrotsko added 2 commits February 26, 2024 20:16

'latest known data' link now preserves context

866ecda

Set default date to the date we have data for all 3 sensors.

63d23f2

dmytrotsko requested a review from melange396 February 26, 2024 18:21

Made Hospital Admissions sensor work as Doctor Visits and Deaths. Rem…

510332f

…oved annotation for Hosptal Admissions sensor when we don't have recent data for choosen date

melange396 requested changes Apr 16, 2024

View reviewed changes

This was referenced Apr 16, 2024

reenabling summary overview widget #1242

Closed

Re-enable summary/Overview.svelte widget on dashboard #1236

Closed

dmytrotsko and others added 2 commits April 17, 2024 16:19

Update src/blocks/NoRecentDataWarning.svelte

96bc531

Co-authored-by: george <george.haff@gmail.com>

Update src/modes/summary/Overview.svelte

8e1ea51

Co-authored-by: george <george.haff@gmail.com>

dmytrotsko requested a review from melange396 April 17, 2024 13:33

dmytrotsko added 6 commits April 17, 2024 16:43

OKRS24-55-UX-default-to-most-recent-data-preserve-user-context Added …

ffbf93f

…comment to explain some split/slice logic in IndicatorWarning

OKRS24-55-UX-default-to-most-recent-data-preserve-user-context Format…

e7f6b7e

…ting

OKRS24-55-UX-default-to-most-recent-data-preserve-user-context Fixed …

7983f45

…typo

OKRS24-55-UX-default-to-most-recent-data-preserve-user-context Added …

14c1f19

…warning message handling.

OKRS24-55-UX-default-to-most-recent-data-preserve-user-context Remove…

b2075f5

…d debug logging

OKRS24-55-UX-default-to-most-recent-data-preserve-user-context Added …

64bf91a

…explanation on split/slice/join operations in NoRecentDataWarning.svelte

melange396 requested changes Apr 23, 2024

View reviewed changes

dmytrotsko and others added 2 commits April 23, 2024 18:31

Update src/blocks/NoRecentDataWarning.svelte

cc1b846

Co-authored-by: george <george.haff@gmail.com>

Update src/blocks/NoRecentDataWarning.svelte

84a1f2b

Co-authored-by: george <george.haff@gmail.com>

melange396 approved these changes Apr 23, 2024

View reviewed changes

OKRS24-55-UX-default-to-most-recent-data-preserve-user-context Format…

a465b1a

…ting

melange396 merged commit 0b42ad1 into dev Apr 23, 2024
6 checks passed

melange396 deleted the OKRS24-55-UX-default-to-most-recent-data-preserve-user-context branch April 23, 2024 15:54

melange396 mentioned this pull request Apr 23, 2024

Release v3.2.10 #1248

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OKRS-24-55 #1243

OKRS-24-55 #1243

dmytrotsko commented Feb 26, 2024 •

edited

Loading

netlify bot commented Feb 26, 2024 •

edited

Loading

melange396 left a comment

melange396 Apr 15, 2024

dmytrotsko Apr 17, 2024

dmytrotsko commented Apr 17, 2024

melange396 left a comment

melange396 Apr 23, 2024

melange396 Apr 23, 2024

melange396 left a comment

melange396 commented Apr 23, 2024

melange396 commented Apr 23, 2024

OKRS-24-55 #1243

OKRS-24-55 #1243

Conversation

dmytrotsko commented Feb 26, 2024 • edited Loading

Summary

netlify bot commented Feb 26, 2024 • edited Loading

✅ Preview link ready!

melange396 left a comment

Choose a reason for hiding this comment

melange396 Apr 15, 2024

Choose a reason for hiding this comment

dmytrotsko Apr 17, 2024

Choose a reason for hiding this comment

dmytrotsko commented Apr 17, 2024

melange396 left a comment

Choose a reason for hiding this comment

melange396 Apr 23, 2024

Choose a reason for hiding this comment

melange396 Apr 23, 2024

Choose a reason for hiding this comment

melange396 left a comment

Choose a reason for hiding this comment

melange396 commented Apr 23, 2024

melange396 commented Apr 23, 2024

dmytrotsko commented Feb 26, 2024 •

edited

Loading

netlify bot commented Feb 26, 2024 •

edited

Loading