[DONT MERGE] chore(EMI-2213): client side signals tracking #15055
Conversation
#1447 Bundle Size — 8.99MiB (+0.05%).9b8161b(current) vs 8385611 main#1444(baseline) Warning Bundle contains 14 duplicate packages – View duplicate packages Bundle metrics
|
| Current #1447 |
Baseline #1444 |
|
|---|---|---|
 Initial JS |
3.66MiB(+0.02%) |
3.66MiB |
 Initial CSS |
0B |
0B |
 Cache Invalidation |
73.01% |
43.88% |
| Chunks |
103 |
103 |
| Assets |
106 |
106 |
| Modules |
5843(+0.02%) |
5842 |
| Duplicate Modules |
530(+0.19%) |
529 |
| Duplicate Code |
4.02% |
4.02% |
| Packages |
266 |
266 |
| Duplicate Packages |
13 |
13 |
Bundle size by type 1 change 1 regression
| Current #1447 |
Baseline #1444 |
|
|---|---|---|
| JS |
8.85MiB (+0.05%) |
8.84MiB |
| Other |
143.36KiB |
143.36KiB |
Bundle analysis report Branch rquartararo/client-side-signals-... Project dashboard
Generated by RelativeCI Documentation Report issue
rquartararo
changed the title
|
|
|
||
| signals?: { [id: string]: string[] } | ||
|
|
||
| updateSignals?: (id: string, newSignals: string[]) => void |
There was a problem hiding this comment.
To summarize the approach, the context maintains a signals data structure that keeps track of all the artworks and their individual collector signals (which can be updated client side in the PrimaryLabelLine component). In the grid/rail item onClick callback, we'll refer to this signals map to fire tracking calls with the correct signals (which may or may not be overwritten on the client side).
| if (label) { | ||
| signals.push(label) | ||
| } | ||
| updateSignals(artwork.internalID, signals) |
There was a problem hiding this comment.
This is the only place we update signals (for tracking purposes). I think it makes sense because this component represents the display of the collector signals to users. We might have to keep in mind if in the future the visual or the exact component implementation changes, we'll update the tracking accordingly.
There was a problem hiding this comment.
#non-blocking. I wonder if we need to use an array to keep track of multiple collector signals for each artwork. Perhaps it can just be a 1:1 mapping, given that we only show 1 primary label.
There was a problem hiding this comment.
Yes right now the signal prioritization logic is in getSignalLabel but we could do it here and we would no longer need an array of signals. I was thinking we might have a separate use case for keeping track of all signals on an artwork in the future? Okay with a 1:1 mapping as well though.
| endAt | ||
| } | ||
| increasedInterest | ||
| curatorsPick |
There was a problem hiding this comment.
Now on the MP schema side, we separate out the fields requiring auth (e.g. partnerOffer) from fields without requiring auth (e.g. primaryLabel), we'll need to request them both.
| /> | ||
| </> | ||
| )} | ||
| <ArtworkGridContextProvider> |
There was a problem hiding this comment.
The same context and the signals data structure are shared among multiple grids/rails, and I think that means when rendering each grid/rail, the same signals will get updated. I guess it's possible that the desired signals of an artwork could be updated back and forth with server-side and client-side signals, but it would eventually settled with the desired signals, right?
The type of this PR is: Chore
This PR solves EMI-2213
Description
This updates the signals tracking to work with client-side loading.
My solution was to use the grid context to get accurate data for client-side signal tracking.
NOTE: We will create a review app to QA tomorrow and then merge at same time as switching the FF.