Stats Review: Add Data Types (P3) - #24715
Conversation
Generated by 🚫 Danger |
|
|
|
||
| static func == (lhs: DataPoint, rhs: DataPoint) -> Bool { | ||
| lhs.date == rhs.date && lhs.value == rhs.value | ||
| } |
There was a problem hiding this comment.
Implement hash(into:) to make Hashable align with the Equtable implementation? If two equal objects should have the same hash code.
There was a problem hiding this comment.
Turns out, Hashable and Equatable is not longer needed for this type. Removed in #24760.
| case comments | ||
| case posts | ||
| case timeOnSite | ||
| case bounceRate |
There was a problem hiding this comment.
Is this bounce rate metric shown in the new Stats? If not, can it be deleted?
There was a problem hiding this comment.
These are supposed to be added in the near future. We've already added localizable strings and everything. I'd usually be in favor of removing the unused code, but I'd rather keep it as it's just the definitions.
| } | ||
|
|
||
| var displayName: String { | ||
| switch sectionName.lowercased() { |
There was a problem hiding this comment.
Is the sectionName localized? If it's not localized, the displayName is also not localized (which should be). If the sectionName is localized, we should not compare it with "author" or "other".
There was a problem hiding this comment.
Good point. I think it's OK if instead of "Authors" (the web shows it) it just says "Author". I removed localizations to keep it simple.
3 participants
This PR introduces data types:
DataPointto represent points on charts (futureChartCard)TopListItemand nested structs to represent types in the top lists (futureTopListCard)The structs have localized titles, icons, etc. Some of the items have small in-memory mock data to be used in production for skeleton views (
.redacted(reason: .placeholder)). It's not the same data as in #24713 (for the full-blown service that covers data over 10+ years).