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

A4A: Marketplace|Products - Make product slider sticky on scroll

Open gogdzl opened this issue 1 year ago • 7 comments

Related to https://github.com/Automattic/automattic-for-agencies-dev/issues/451

Proposed Changes

  • This PR moves the bundle size selection slider to the page header, as requested in https://github.com/Automattic/automattic-for-agencies-dev/issues/451. The header's position remains unchanged, as it's already sticky, the slider also respects this behavior.

  • When the screen size is not big enough to accommodate the slider within the header, it is moved down and remains sticky.

Testing Instructions

  • Navigate to the products section of the Marketplace (http://agencies.localhost:3000/marketplace/products)
  • Verify that the slider is positioned according to https://github.com/Automattic/automattic-for-agencies-dev/issues/451
  • Verify that the slider is sticky
  • Shrink down the page
  • Verify that the slider always respects borders and does not overflow
  • Verify that both sliders are in sync and always show the same bundle size

image

image

image

Pre-merge Checklist

  • [ ] Has the general commit checklist been followed? (PCYsg-hS-p2)
  • [ ] 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)?
  • [ ] For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-aUh-p2)?

gogdzl avatar May 10 '24 07:05 gogdzl

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

Sections (~87 bytes added 📈 [gzipped])

name                          parsed_size           gzip_size
a8c-for-agencies-marketplace       +479 B  (+0.1%)      +89 B  (+0.1%)
sites-dashboard                     +14 B  (+0.0%)       -1 B  (-0.0%)
site-monitoring                     +14 B  (+0.0%)       -1 B  (-0.0%)
hosting                             +14 B  (+0.0%)       -1 B  (-0.0%)
github-deployments                  +14 B  (+0.0%)       -1 B  (-0.0%)
dev-tools-promo                     +14 B  (+0.0%)       -1 B  (-0.0%)
a8c-for-agencies-sites              +14 B  (+0.0%)       -1 B  (-0.0%)
a8c-for-agencies-referrals          +14 B  (+0.0%)       -1 B  (-0.0%)
a8c-for-agencies-purchases          +14 B  (+0.0%)       -1 B  (-0.0%)
a8c-for-agencies-plugins            +14 B  (+0.0%)       -1 B  (-0.0%)
a8c-for-agencies-overview           +14 B  (+0.0%)       -1 B  (-0.0%)
a8c-for-agencies-landing            +14 B  (+0.0%)       -1 B  (-0.0%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

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 May 10 '24 07:05 matticbot

This mostly looks good to me, but I think it's a bit different behavior from the original request. For me, I see the slider always sticky in the header, instead of "jumping" there on scroll. However, It doesn't look bad and I would leave it for @jeffgolenski to comment wether we should implement the originally suggested behavior or keep this one.

Thanks for the saying that it doesn't look bad. I went for this approach because when testing a new component that I would use to track visibility, I thought it looked nice this way. That's also the reason I requested @jeffgolenski and @keoshi as reviewers.

gogdzl avatar May 10 '24 18:05 gogdzl

@gogdzl I looks quite good 💯

andrii-lysenko avatar May 10 '24 20:05 andrii-lysenko

@gogdzl - Great work so far my friend. Just a heads up, we need to add an additional element to the header for the automated referrals project. It may be best to wait on us to move forward with those designs before merging this.

We just need to figure out how everything is going to seamlessly work together, so we don't have to make too many necessary changes here. That okay? cc @madebynoam @keoshi

jeffgolenski avatar May 13 '24 16:05 jeffgolenski

Any update on this? Thank you! @jeffgolenski @madebynoam @keoshi

gogdzl avatar May 20 '24 17:05 gogdzl

Any update on this? Thank you! @jeffgolenski @madebynoam @keoshi

Hey @gogdzl! Thanks for waiting on this. @jeffgolenski and I will work on an refining the design in the next day or and ping you. We want to make sure all the additions (product filters and referral mode) will fit in.

madebynoam avatar May 23 '24 10:05 madebynoam

Hey @gogdzl and @andrii-lysenko, @rcanepa,

@keoshi, @jeffgolenski, and I worked on how the control will stick in regards to all the new additions to the header. As a result we've also come up with an updated design for the slider that is more structured. I've made a quick video on how we want it to stick in the header:

https://github.com/Automattic/wp-calypso/assets/8889977/3e650769-7e8a-4f7e-b31b-9fc863bb9293

Do you have any recommendation on how we should proceed to incorporate the updated control and the sticking behavior?

madebynoam avatar May 28 '24 10:05 madebynoam

@madebynoam I think this is a great next step. RE: IxD, I think it feels great.

Can we also mock up a quick prototype for the WordPress.com & Pressable plans UI to stress test it?

WPCOM

  • For brand new purchases (agency has 0 wpcom plans)
  • For recurring wpcom purchases (agency has 12 wpcom plans already)

Pressable:

  • For a brand new Pressable plan selection
  • For upgrading or downgrading their current plan

If we need to, I'm happy to spin up a post-launch project for this, as I'd rather make sure we have a truly scalable pattern we only have to alter once.

jeffgolenski avatar May 29 '24 21:05 jeffgolenski

@madebynoam Looks great and doable. I think we are safe to close this PR and create a brand new one. Should we move this discussion to a P2?

gogdzl avatar May 29 '24 22:05 gogdzl

Update: I've created a new project for this: https://github.com/Automattic/automattic-for-agencies-dev/issues/646

jeffgolenski avatar Jun 07 '24 20:06 jeffgolenski

Update: I've created a new project for this: Automattic/automattic-for-agencies-dev#646

Closing this PR.

gogdzl avatar Jun 17 '24 09:06 gogdzl