Dashboard: Telemetry Opt In Banner #4374
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
dmmulroy
requested review from
swissspidy,
carloskelly13,
BrittanyIRL,
littlemilkstudio and
mariano-formidable
|
Size Change: +2.65 kB (0%) Total Size: 1.25 MB
ℹ️ View Unchanged
|
swissspidy
reviewed
Sep 8, 2020
…-stories-wp into feature/4226-telemetry-banner
carloskelly13
approved these changes
Sep 8, 2020
| if ( | ||
| bannerPreviouslyClosed || // The banner has been closed before | ||
| currentPath === APP_ROUTES.EDITOR_SETTINGS || // The user is on the settings page | ||
| !dataIsLoaded || // currentUser is not loaded yet |
There was a problem hiding this comment.
This is good to not show the banner prematurely if the user data hasn't come in yet.
|
Tests and functionality works. I think if we want to tie the dismissal of the banner to the user's persisted state over localStorage in the API then that would be a nice follow on evolution. Also tested the Settings page too and that works well. |
Codecov Report
@@ Coverage Diff @@
## main #4374 +/- ##
==========================================
- Coverage 83.47% 83.43% -0.04%
==========================================
Files 862 864 +2
Lines 15118 15165 +47
==========================================
+ Hits 12619 12653 +34
- Misses 2499 2512 +13
Flags with carried forward coverage won't be shown. Click here to find out more.
|
BrittanyIRL
approved these changes
Sep 9, 2020
| @@ -0,0 +1,41 @@ | |||
| /* | |||
There was a problem hiding this comment.
nit: typo on this file name (telemetyBanner) - missing the r
swissspidy
added
Group: Dashboard
Group: Telemetry
Pod: Pea (Dashboard & Templates)
Type: Enhancement
Non-admins need to be able to update their telemetry opt-in preferences
swissspidy
approved these changes
Sep 10, 2020
…-stories-wp into feature/4226-telemetry-banner
BrittanyIRL
approved these changes
Sep 10, 2020
Comment on lines
+115
to
+118
| if (canManageSettings) { | ||
| fetchSettings(); | ||
| } | ||
| }, [fetchSettings, canManageSettings]); |
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.
Summary
This adds an opt in banner for telemetry tracking that will display until the user either closes it or interacts with the telemetry opt in checkbox on the
Edit Settingspage/view. It will be visible on all top level views using thePageHeadershared component with the exception of theEdit Settingspage/view.Relevant Technical Choices
useTelemetryOptInhook so as not to duplicate logic between the banner and settings page.telemetryprops to thePageHeadercomponent and end up editing all of our top level views.@testing-library/user-eventsdependency to circumvent the following issue: onChange is called when checkbox is disabled testing-library/react-testing-library#275MockApiProviderandrenderWithProviderutils to make view integration tests easier.User-facing changes
Editor Settings page, the banner should not display again. In addition, when opting in using the banner the title should change toYour selection has been updated. Thank you for helping to improve the editor!.Testing Instructions
Your selection has been updated. Thank you for helping to improve the editor.and unchecking it reverts it back.#4226