wp-calypso icon indicating copy to clipboard operation
wp-calypso copied to clipboard

Limit and shuffle generated designs in UDP

Open miksansegundo opened this issue 2 years ago • 5 comments

Proposed Changes

  • Shuffle the generated designs using lodash
  • Limit them to three using a constant in the frontend. I think is here because we want to get three randomized from the list of published recipes in that category.

After merging this PR, we will publish the recipe BLOG LAYOUT 4.

Testing Instructions

  • Create a new site, choose goals, and a vertical to get to the design picker step
  • Check that we only display three generated designs when more are published. We can test locally by changing the value of the constant GENERATED_DESIGNS_LIMIT to 2 and testing that only two are displayed because if we publish more recipes they will be available in production.
Screen Shot 2565-09-22 at 17 15 29

Pre-merge Checklist

  • [ ] Have you written new tests for your changes?
  • [ ] Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
  • [ ] Have you checked for TypeScript, React or other console errors?
  • [ ] Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
  • [ ] Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?

Related to #115

miksansegundo avatar Sep 22 '22 10:09 miksansegundo

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

App Entrypoints (~155 bytes added 📈 [gzipped])

name           parsed_size           gzip_size
entry-stepper       +801 B  (+0.0%)     +155 B  (+0.0%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory. Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

matticbot avatar Sep 22 '22 10:09 matticbot

@miksansegundo -- we have already shuffled the designs in the backend side (the starter-designs endpoint), using a pre-defined randomization seed (based on the site url). Adding another layer of randomization/shuffling in Calypso would break that logic.

Can we instead, add this logic in the backend as well?

fushar avatar Sep 22 '22 10:09 fushar

@fushar I thought that others could be using that endpoint. I'll remove the logic for shuffle. For the limit, how do you suggest me to do it?

miksansegundo avatar Sep 22 '22 10:09 miksansegundo

Oh, that's a good point. Hmm. For limit, I think either works (putting in on wpcom or Calypso).

If we want to do it on Calypso side, we can add the logic to select the first 3 designs from the generated designs in this block:

https://github.com/Automattic/wp-calypso/blob/c62b539d1c8b613b0dcfb74c9323b3dfedadca05/client/landing/stepper/declarative-flow/internals/steps-repository/design-setup/unified-design-picker.tsx#L82-L97

Not sure. But I slightly lean towards putting this logic on wpcom as well, so that we are not returning any data to Calypso other than what we need. Also, the only consumer of the starter-designs endpoint is only us (right now). Maybe later if someone wants to use this endpoint as well, we can add a query param to the endpoint to limit the number of generated designs.

Thoughts? @Automattic/ganon

fushar avatar Sep 22 '22 10:09 fushar

Moving the shuffling and limit logic to wpcom seems OK to me 👍

taipeicoder avatar Sep 23 '22 01:09 taipeicoder