fix(llm): add tracking on pull to refresh #7025
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 5 Ignored Deployments
|
LucasWerey
force-pushed
the
fix/LIVE-12572
branch
from
1597e39
to
9ad1fcd
Compare
LucasWerey
force-pushed
the
fix/LIVE-12572
branch
from
9ad1fcd
to
7c8b44a
Compare
| @@ -24,6 +25,7 @@ export default <P,>(ScrollListLike: React.ComponentType<P>) => { | |||
| const [refreshing, setRefreshing] = useState(false); | |||
|
|
|||
| function onPress() { | |||
| track("buttonClicked", { button: "pull to refresh" }); | |||
There was a problem hiding this comment.
It might be better to add this as a const to the analytics file, and then import it, as it is repeated many times, and allows us to be consistent at naming technics ie. prefix / suffix etc.
However, saying that it seems there is no analytics const file in ll, so may be overkill.
There was a problem hiding this comment.
I thought about it but as there's no dedicated file I didn't do it.
But it might be a good thing to have a file that groups all the track event hooks, as we often use track(“button_clicked”) and others.
There was a problem hiding this comment.
exactly, this makes it confusing to have multiple selectors that semantically have the same meaning but just a different syntax.
However, agree this may be outside the scope of your PR.
| const [refreshControlVisible, setRefreshControlVisible] = useState(false); | ||
| const handlePullToRefresh = useCallback(() => { | ||
| refresh(); | ||
| setRefreshControlVisible(true); | ||
| }, [refresh]); | ||
| trackPullToRefresh(); | ||
| }, [refresh, trackPullToRefresh]); |
There was a problem hiding this comment.
does this list need to be exhausted? given trackPullToRefresh is static?
There was a problem hiding this comment.
I added it to avoid lint error but yes it might not be useful
There was a problem hiding this comment.
You can add this comment above the line to ignore the lint requirement.
| }, [refresh, trackPullToRefresh]); | |
| }, | |
| // eslint-disable-next-line react-hooks/exhaustive-deps | |
| [refresh] | |
| ); |
✅ Checklist
npx changesetwas attached.📝 Description
add track on pull to refresh where a sync is triggered or not so we can quantify the amount of Sync requested to the backend
Screen.Recording.2024-06-06.at.12.02.27.mov
❓ Context
🧐 Checklist for the PR Reviewers