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

A4A: Use a regular Automattic logo color variation in the 'Add new site' split button.

Open jkguidaven opened this issue 1 year ago • 3 comments

The current split button features the Automattic logo, which is difficult to see when not selected. This pull request addresses this issue by using the regular variation color.

Before After
Screenshot 2024-05-24 at 4 44 45 PM Screenshot 2024-05-24 at 4 44 30 PM

Follow up https://github.com/Automattic/wp-calypso/pull/90883

Proposed Changes

Why are these changes being made?

Testing Instructions

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)?

jkguidaven avatar May 24 '24 08:05 jkguidaven

This PR modifies the release build for the following Calypso Apps:

For info about this notification, see here: PCYsg-OT6-p2

  • wpcom-block-editor

To test WordPress.com changes, run install-plugin.sh $pluginSlug fix/a4a/split-button-a4a-icon on your sandbox.

matticbot avatar May 24 '24 08:05 matticbot

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

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

name           parsed_size           gzip_size
entry-stepper        +54 B  (+0.0%)      +11 B  (+0.0%)
entry-main           +54 B  (+0.0%)      +11 B  (+0.0%)
entry-login          +54 B  (+0.0%)      +11 B  (+0.0%)

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

Sections (~36 bytes removed 📉 [gzipped])

name                       parsed_size           gzip_size
a8c-for-agencies-sites           -39 B  (-0.0%)      -19 B  (-0.0%)
a8c-for-agencies-overview        -39 B  (-0.0%)      -17 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 24 '24 08:05 matticbot

I've noticed that if you click on the Add new site button directly, the zip downloads directly. This is a bit weird because, as a user, I know nothing about it.

If I click in the collapsable chevron icon, this is what it looks like now:

image

cc: @keoshi / @madebynoam

cleacos avatar May 24 '24 15:05 cleacos

I've noticed that if you click on the Add new site button directly, the zip downloads directly. This is a bit weird because, as a user, I know nothing about it.

Good call out @cleacos - is there a way to make the default action just open the options in this case as a quick solution?

@jkguidaven - Apologies, but now that I'm testing it, I feel like the fill color within the pill shape of the Automattic logo should change.

  • During resting state when the entire link has a white background, keep the dark pill shape
  • On hover, when the link background is blue, have the white pill shape.

CleanShot 2024-05-24 at 16 33 24

CleanShot 2024-05-24 at 16 33 48@2x

jeffgolenski avatar May 24 '24 20:05 jeffgolenski

Thanks everyone for the review ❤️

Apologies, but now that I'm testing it, I feel like the fill color within the pill shape of the Automattic logo should change.

During resting state when the entire link has a white background, keep the dark pill shape On hover, when the link background is blue, have the white pill shape.

This has been addressed 👍

https://github.com/Automattic/wp-calypso/assets/56598660/c7f3a0b4-13cc-4761-8fd0-d8ef8b182447

is there a way to make the default action just open the options in this case as a quick solution?

@jeffgolenski I couldn't find a quick solution for this because the Calypso's split button component wasn't designed to do this. It will require some refactoring, which has some risk of breaking other areas that rely on the same component.

My suggestion for a quick fix would be to disable the main button so that it doesn't trigger any action when clicked or use a regular dropdown menu. wdyt?

jkguidaven avatar May 27 '24 12:05 jkguidaven

Nice stuff — maybe we can iterate on this further by:

  • Changing the button to say “With the A4A Plugin”.
  • Having a modal pop up after clicking the button.
  • The modal explains what the plugin is and what it does.
  • The modal also contains a link to the direct download.

@madebynoam Had a really nice modal design that is close to the plugin UI, and could be a great way to bridge the plugin and the platform.

keoshi avatar May 27 '24 14:05 keoshi

Nice stuff — maybe we can iterate on this further by:

  • Changing the button to say “With the A4A Plugin”.
  • Having a modal pop up after clicking the button.
  • The modal explains what the plugin is and what it does.
  • The modal also contains a link to the direct download.

@madebynoam Had a really nice modal design that is close to the plugin UI, and could be a great way to bridge the plugin and the platform.

I like this idea. If you can share the link to the modal design, I can take a look 😃 . I feel like this is a feasible solution in a short period of time.

jkguidaven avatar May 28 '24 14:05 jkguidaven

@jkguidaven Here's the modal that @madebynoam designed for Referrals:

image

Figma link here: https://www.figma.com/design/fuufP6VNfZXYmvLsTRWRlG/Referrals?node-id=7145-73019&t=HUhvlc6oeRLm3N85-11

Worth noting though that this issue is slightly related to a bigger effort of letting customers add sites in multiple ways:

  • Creating new site (Dotcom or Pressable).
  • Adding an existing site via the A4A plugin.
  • Adding an existing site via Jetpack connection.
  • Importing existing sites from Dotcom.

We're tracking this here: https://github.com/Automattic/automattic-for-agencies-dev/issues/562

Over the next couple of days I'm hoping to systematize this “Add site” flow a little bit more, so we can ship this PR and address that bigger push later. What do you think?

keoshi avatar May 28 '24 17:05 keoshi

@jkguidaven

My suggestion for a quick fix would be to disable the main button so that it doesn't trigger any action when clicked or use a regular dropdown menu. wdyt?

Works for me if that's an immediate solution we can ship before Noam and Filipe finalize their concepts mentioned above?

jeffgolenski avatar May 29 '24 21:05 jeffgolenski

Over the next couple of days I'm hoping to systematize this “Add site” flow a little bit more, so we can ship this PR and address that bigger push later. What do you think?

Yes, I'd love to have a proper way to address this. Will wait on it 👍

jkguidaven avatar May 30 '24 09:05 jkguidaven

Works for me if that's an immediate solution we can ship before Noam and Filipe finalize their concepts mentioned above?

Sounds good to me. 👍

jkguidaven avatar May 30 '24 09:05 jkguidaven

@jkguidaven Sorry my friend, I didn't anticipate this working quite like this due to the browser functionality. Can we revert this change? With the disabled state + the cursor change, this is communicating that the user isn't able to add new sites at all, even though the chevron is active. It's a confusing state. Sorry about that.

We'll just have to prioritize the modal selection screen Filipe mentioned above cc @rcanepa

CleanShot 2024-05-30 at 14 01 38

jeffgolenski avatar May 30 '24 18:05 jeffgolenski

@jeffgolenski, I created a PR that reverts this one: https://github.com/Automattic/wp-calypso/pull/91310

rcanepa avatar May 30 '24 18:05 rcanepa

@jeffgolenski, I apologize for going ahead and merging this without your final confirmation. Thanks @rcanepa for creating the revert PR 👍

jkguidaven avatar Jun 01 '24 03:06 jkguidaven

@jkguidaven, mistakes like this will happen again in the future, and that's okay. I wouldn't like you or anyone else to stop shipping things without Jeff's or someone else's approval! I'm sure Jeff wouldn't like it either!

Next time, I suggest sharing a screenshot or recording of the behavior so stakeholders can better understand the changes' impact.

You're shipping a lot of great work and with great speed. Keep doing it!

rcanepa avatar Jun 03 '24 20:06 rcanepa

@rcanepa +1000!

keoshi avatar Jun 03 '24 21:06 keoshi