-
Notifications
You must be signed in to change notification settings - Fork 710
For #5519: Add telemetry for theme settings. #5526
Conversation
Request for data collection review formAll questions are mandatory. You must receive review from a data steward peer on your responses to these questions before shipping new data collection. 1) What questions will you answer with this data? 2) Why does Mozilla need to answer these questions? Are there benefits for users? Do we need this information to address product or business requirements? 3) What alternative methods did you consider to answer these questions? Why were they not sufficient? 4) Can current instrumentation answer these questions? 5) List all proposed measurements and indicate the category of data collection for each measurement, using the Firefox data collection categories found on the Mozilla wiki. Note that the data steward reviewing your request will characterize your data collection based on the highest (and most sensitive) category.
6) How long will this data be collected? Choose one of the following: 7) What populations will you measure? 8) If this data collection is default on, what is the opt-out mechanism for users? 9) Please provide a general description of how you will analyze this data. 10) Where do you intend to share the results of your analysis? 12) Is there a third-party tool (i.e. not Telemetry) that you are proposing to use for this data collection? |
Data Review
Yes, through the metrics.yaml and the Glean Dictionary
Yes, through the "Send Usage Data" preference in the application settings.
N/A, the collection is set to end or be renewed by 2022-10-01
Category 2, User interaction
default-on
No
Yes
No Resultdata-review+ |
// theme telemetry | ||
val currentTheme = | ||
when { | ||
settings.lightThemeSelected -> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated to this PR: I never understood in Fenix why we have separate prefs for a radio group. Why one one pref with three values?
It completely skipped my mind to comment on this in the light mode PR when it was in review. 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because radio preferences do not really exist. The only grouping made in native preferences is in list preferences. So we have to use checkbox preferences and add the grouping in our code.
// theme telemetry | ||
val currentTheme = | ||
when { | ||
settings.lightThemeSelected -> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mcarare Hey I am doing the same for iOS and I noticed that in metrics.yaml
these are all in uppercase FOLLOW DEVICE
, LIGHT
, DARK
while you set them here with mixed case. I will follow the code for consistency.
@travis79 Glean will not uppercase these strings right? They will appear in the database like they are spelled out in the code here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is correct. Glean will not modify the recorded strings, they will maintain the same case as they were recorded with.
No description provided.