feature: add HaTS stuff to localStorage #8040
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
beyackle
requested review from
a-b-r-o-w-n,
benbrown,
boydc2014,
cwhitten,
GeoffCoxMSFT,
hatpick,
pavolum,
srinaath,
tdurnford and
tonyanziano
as code owners
tdurnford
requested changes
Jun 9, 2021
Composer/packages/client/src/App.tsx
Outdated
| @@ -38,6 +45,19 @@ export const App: React.FC = () => { | |||
| ipcRenderer?.on('cleanup', (_event) => { | |||
| performAppCleanupOnQuit(); | |||
| }); | |||
|
|
|||
| let days = surveyStorage.get('days', 0); | |||
There was a problem hiding this comment.
All of this logic should be handled in the setSurveyEligibility dispatcher. You should just call the dispatcher on mount and let it determine how and if it should update the value.
There was a problem hiding this comment.
Good idea. I'll fix that.
Composer/package.json
Outdated
| @@ -20,7 +20,9 @@ | |||
| "url-parse": "^1.5.1", | |||
| "underscore": "^1.12.1", | |||
| "dns-packet": "^1.3.4", | |||
| "json-ptr": "^2.1" | |||
| "json-ptr": "^2.1", | |||
There was a problem hiding this comment.
Are these dependencies necessary? They seem out of scope for this PR.
There was a problem hiding this comment.
They were causing a security alert, but I've now handled that in a separate PR which has merged already (so these can/will go away on a rebase).
tdurnford
reviewed
Jun 10, 2021
| @@ -525,3 +525,11 @@ export const defaultTeamsManifest: TeamsManifest = { | |||
|
|
|||
| export const defaultBotPort = 3979; | |||
| export const defaultBotEndpoint = `http://localhost:${defaultBotPort}/api/messages`; | |||
|
|
|||
| const DAYS = 1000 * 60 * 60 * 24; // ms per day | |||
There was a problem hiding this comment.
Suggested change
| const DAYS = 1000 * 60 * 60 * 24; // ms per day | |
| const MS_PER_DAY = 1000 * 60 * 60 * 24; |
There was a problem hiding this comment.
I thought about that, but I like the way it makes "90 * DAYS" legible a few lines farther down.
tdurnford
approved these changes
Jun 10, 2021
benbrown
pushed a commit
that referenced
this pull request
Jun 11, 2021
* add stuff to localStorage * add stuff to localStorage * move logic and add more checks * Update constants.tsx
beyackle
added a commit
that referenced
this pull request
Jun 29, 2021
* feature: add HaTS stuff to localStorage (#8040) * add stuff to localStorage * add stuff to localStorage * move logic and add more checks * Update constants.tsx * create prototype card * add congratulation notification * feature: show survey card on Home page (#8078) * add custom style * Update Publish.tsx * display card on Home page * Update constants.tsx * Update shell.ts * Update Home.tsx * move survey into separate component * get more info automatically * move URL into constant * move more props around * Update constants.tsx * turn non-rendering component into hook * move card to design page * start adding ipcRenderer stuff * update card design * get machineID and stash in Recoil * read OS and place in URL * add spacer to notification card * better notification styling * fix constants and typercheck error * Revert "feature: show survey card on Home page (#8078)" (#8149) This reverts commit 5b9f164. * feature: add new survey notification to design page (#8155) * add custom style * Update Publish.tsx * display card on Home page * Update constants.tsx * Update shell.ts * Update Home.tsx * move survey into separate component * get more info automatically * move URL into constant * move more props around * Update constants.tsx * turn non-rendering component into hook * move card to design page * start adding ipcRenderer stuff * update card design * get machineID and stash in Recoil * read OS and place in URL * add spacer to notification card * better notification styling * fix constants and typercheck error * fixes from PR * fix card alignment and make aka.ms URL * remove redundant prop * use query-string package to generate link * move MAC truncation to electron server * add fixes from CR and machineID truncation * fix typechecking * Update ToolbarButtonMenu.test.tsx * Update App.tsx * add custom onDismiss to cards * restore delete-on-click for links * Update NotificationCard.tsx * add comments from PR * revert ToolbarButtonMenu test * Update NotificationCard.tsx * add comment about truncation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This addresses the issue of storing things on the client so we know when to display the HaTS survey; it will keep track, in local storage, of the number of unique calendar days that a user uses Composer. If this exceeds 5, it sets a flag in the Recoil store that can be read in a later step (to be written) to see if we should display the survey. Once the survey has been taken, we can use a similar mechanism to store the latest date the survey was taken and make sure we aren't showing the link to a user who's taken it in the last 90 days.
Task Item
refs #8029
Screenshots
Not really a UI change, but here are the (currently) two new values in localStorage.
