Count active users with Google Analytics #112
Open
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
The "Dismiss this header" button is not included in the popup, for obvious reasons. The "Restore original content" button is fully working. The message remains the same after restoring content.
The "Dismiss this header" button is not included in the popup, for obvious reasons. The "Restore original content" button is fully working. The message remains the same after restoring content.
Also fixed "Restore original content" button in popup.
This uses a hardcoded popup URL, which should be changed.
The extension ID is also static now.
With appropriate end-to-end tests
When the popup requests the extension status from the content script, it now receives a keyword telling it which message to display. It previously received the entire message that it should display. The Status enum can't be imported from `popup.js`, so a temporary solution is used.
Remove tests for `getExcludedDomain` and `getWhyExcluded`
This reverts commit 2694fc2. The removed tests should be left in, as the functions they are testing will be put back in. Tests will not pass on this commit.
If the extension had to be disabled on a particular site, the header would explain why. Now the popup explains why.
A broken test was found and fixed as part of this commit.
The "Dismiss this header" button is not included in the popup, for obvious reasons. The "Restore original content" button is fully working. The message remains the same after restoring content.
The "Dismiss this header" button is not included in the popup, for obvious reasons. The "Restore original content" button is fully working. The message remains the same after restoring content.
Also fixed "Restore original content" button in popup.
This uses a hardcoded popup URL, which should be changed.
Remove tests for `getExcludedDomain` and `getWhyExcluded`
This reverts commit 2694fc2. The removed tests should be left in, as the functions they are testing will be put back in. Tests will not pass on this commit.
If the extension had to be disabled on a particular site, the header would explain why. Now the popup explains why.
A broken test was found and fixed as part of this commit.
This one isn't tracking for some reason.
This is used to get an accurate count of how many users actively browse with the extension.
We only want a user count for now.
|
I see the same duplicate commits here as with #113 and I'd suggest I similar course of action. I'm sorry I got so far behind! |
|
Squash merge results in a reasonable set of changed files here also, though if you want a proper review through GitHub it would be helpful to have a clean PR with a minimal set of commits and changed files. |
commit a7f3706fca9bb459673f495b47d42478145aa00e
Author: Janet Davis <davisj@whitman.edu>
Date: Mon Nov 25 15:20:14 2019 -0800
Added notes from #96 to manual test cases
commit debd4ba38450d0736cdb58cc29e74df53f993351
Author: Ian Hawkins <ian@hawkapp.com>
Date: Thu Nov 14 15:44:14 2019 -0800
Make icon grey in more sizes
commit fbe703f868f586228af60e66d49e64a8b37bf3d1
Author: Ian Hawkins <ian@hawkapp.com>
Date: Sat Nov 9 09:19:11 2019 -0800
Grey out icon when it makes sense
This doesn't work for all icon sizes.
|
@ProfJanetDavis Can we close this PR, or do you want to use this Analytics experiment? |
|
Good question. After our last meeting, I am not sure Google Analytics is
going to serve my purposes for evaluation - I need to investigate more.
Let's leave it for now. At some point we may close the PR but leave the
branch as evidence of what you learned.
…On Thu, Jan 16, 2020 at 10:08 PM Ian Hawkins ***@***.***> wrote:
Reopened #112 <#112>.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#112?email_source=notifications&email_token=AEXJIU4GYV5YYDIC6ZVLJ7TQ6FDOPA5CNFSM4JMMBQFKYY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGOWBAHYVA#event-2957016148>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEXJIU3AMSFIFY6DVXWSIZDQ6FDOPANCNFSM4JMMBQFA>
.
--
Janet Davis (she/her or they/them)
Associate Professor and Microsoft Chair of Computer Science, Whitman
College
509-527-5758, http://cs.whitman.edu/~davisj/
Schedule an appointment: http://meetme.so/JanetDavis
|
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.
Resolves #53