[DONT MERGE] feat: load limited-time offer signals on the client side #14897
Conversation
This depends on MP change that removes partner offers from primary labels. Primary label should only consist of unauthenticated data.
Now we have to consider partner offer (that's loaded on the client side) when displaying "increased interest" and "curtors' pick" badges (that can be loaded on the server and cached), it may visually look better moving the limited-time offer back to the first line, with the badges. While there will be some content shift, we can preserve the original collector signals design (vs. partially porting over the app design).
|
#1007 Bundle Size — 9.57MiB (+0.25%).da81774(current) vs ffe4f84 main#484(baseline) Important Bundle introduced 1 and removed 4 duplicate packages – View changed duplicate packages Warning Bundle introduced 3 new packages: web-vitals, @sentry-internal/browser-utils, stylis – View changed packages Bundle metrics
|
| Current #1007 |
Baseline #484 |
|
|---|---|---|
 Initial JS |
3.7MiB(-6.53%) |
3.95MiB |
 Initial CSS |
0B |
0B |
 Cache Invalidation |
87.82% |
2.04% |
| Chunks |
142(-0.7%) |
143 |
| Assets |
146 |
146 |
| Modules |
5701(+1.14%) |
5637 |
 Duplicate Modules |
501(+10.11%) |
455 |
| Duplicate Code |
6.12%(+4.08%) |
5.88% |
| Packages |
282(-3.09%) |
291 |
| Duplicate Packages |
39(-7.14%) |
42 |
Bundle size by type 2 changes 1 regression 1 improvement
| Current #1007 |
Baseline #484 |
|
|---|---|---|
| JS |
9.36MiB (+0.46%) |
9.31MiB |
| Other |
218.23KiB (-8.25%) |
237.84KiB |
Bundle analysis report Branch review-app-client-side-collector... Project dashboard
Generated by RelativeCI Documentation Report issue
| ) | ||
| } | ||
|
|
||
| const SaleMessageFragmentContainer = createFragmentContainer(SaleMessage, { |
There was a problem hiding this comment.
#nonblocking and harmless, but would def encourage all new relay code to be written using hooks:
const data = useFragment(...)Its a bit nicer from an import perspective too; can do import { SaleMessage } from vs having to mention fragment containers.
| relayEnvironment: mockEnvironment, | ||
| } | ||
| }) | ||
| }) |
There was a problem hiding this comment.
#non-blocking (but recommended): Rather than mocking out all of this stuff, can simply test the fragment via our relay testing tools and skip all of the qury renderer wiring (which historically we typically don't test):
const { renderWithRelay } = setupTestWrapperTL({
Component: SaleMessageFragmentContainer
query: ...
})
it('works', () => {
renderWithRelay(someFixture)
expect(screen.getByText("foo")).toBeOnScreen()
})
| ) | ||
|
|
||
| act(() => { | ||
| mockEnvironment.mock.resolveMostRecentOperation(operation => |
There was a problem hiding this comment.
This allows us to skip manually mocking both the test environment and the relay operation; all of those things happen inside of our test tools automatically.
The type of this PR is: Feat
DO NOT MERGE YET. This depends on artsy/metaphysics#6220, and we might want to include tracking changes.
This PR solves EMI-2161
Description
In the previous attempt, we tried a few design options to display the limited-time offer signals on the client-side. We then realized the signal has to be considered together with other badge signals (i.e. "increased interest" and "curators' pick") because we only want to display at most one. So we decided to switch back to the original signals design and let client-side loaded signals, if any, dynamically replace server-side rendered content. We now think it's acceptable to have content shift for this particular data which is relatively lower volume.
This is an example that the client-side loaded limited-time offer signal replaces an increased interest badge in-place. We can see the offered price also replaces the listed price shortly after page loads, as well as the timer and save state pops in.
Recording
This is an example when without other badges, the limited-time offer badge is inserted which causes some content shift.
Recording
cc @artsy/emerald-devs