Hosting Dashboard V2: Add notice component

[arthur791004]

Part of DOTCOM-13192

Proposed Changes

• Add the Notice component for use on the Transfer site.

Variant

Info	Warning	Success	Error

Density

Low	Medium	High

Others

Not dismissible	Without actions

Why are these changes being made?

• Run yarn run storybook:start
• Go to http://localhost:6006/?path=/story/client-dashboard-notice--default

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 UI changes, have we tested the change in various languages (for example, ES, PT, FR, or DE)? The length of text and words vary significantly between languages.
• 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)?

[github-actions]

Calypso Live (direct link)

https://calypso.live?image=registry.a8c.com/calypso/app:build-145671

Jetpack Cloud live (direct link)

https://calypso.live?image=registry.a8c.com/calypso/app:build-145671&env=jetpack

Automattic for Agencies live (direct link)

https://calypso.live?image=registry.a8c.com/calypso/app:build-145671&env=a8c-for-agencies

[arthur791004]

@matt-west I noticed that the warning icon differs from the one used in Gutenberg. Was this intentional? Thank you!

[matticbot]

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

[youknowriad]

Feels like we have too much "Notice" components, we should get some clarity on what to use where and maybe adapt naming.

[jasmussen]

Feels like we have too much "Notice" components, we should get some clarity on what to use where and maybe adapt naming.

This is meant to be a replacement for the Notice component. Not a new separate component.

CC: @fcoveram as you worked on this design.

[fcoveram]

Yes. As Joen said, it was thought to replace the current one.

The variants are specs of the same component. The four main variants are voice tones that exist with the content density. You can see how the latter works in the Figma documentation.

[arthur791004]

You can see how the latter works in the dhW7zYBamXJIeJIE3wy5f4-fi-3765_38524.

Oh nice! I'll update the component based on the Figma spec 🙂

[ciampo]

Given the different notice densities, does it make sense at all to use Card and its subcomponents here vs just a div? Alternatively, should we look into updating Card to offer the same density (padding) options?

cc @jasmussen @fcoveram

[jameskoster]

Card already has a size prop which is pretty similar to the density options here. Could/should we use that?

[ciampo]

We could, but as you noticed, it may not follow the exact specs needed by Notice — that's why I was asking.

[arthur791004]

Yeah, the padding on different Card sizes here doesn't match the padding used in the density option here 🥲

[arthur791004]

It would be better if we could update the Card, CardHeading, and CardBody components to match the new design (or the styles of the new components), or consider creating a new unified one. Otherwise, we'll need to manually customize padding, font sizes, and other styles each time we implement a new component with a similar structure.

[fcoveram]

The padding values consider the spacings inside the icon. So the difference is intentional to balance the layout in all density variants. Below you can see a quick test using the values applied in the design, and the ones inherited from Card.

Design paddings

The alignment is slightly off, but general spacings are balanced according to the page layout.

Card paddings.

The component's side spacings make it look more prominent.

[jameskoster]

Can't we use the closest Card size then add margin/padding to the internal notice container to make up the difference? So for example if we want 24px padding, use card size small (16px padding) then add 4px padding to the notice container? Having typed that out I guess there's not much value 🙂

I suppose in an ideal world Card size presets would be a bit more expansive than they are currently.

[ciampo]

We can, probably not much of a difference with overriding Card's styles — the issue would be the same: if Card updates, this component would also need updating to avoid visual regressions

[arthur791004]

Yes

[jameskoster]

I suppose the long-term solution would be to utilise some semantic spacing tokens from the Theme package.

[jameskoster]

Wasn't the idea to build this as an update to the Notice component in Calypso?

[matt-west]

@matt-west I noticed that the warning icon differs from the one used in Gutenberg. Was this intentional?

Yes, I think @fcoveram added this in his redesign of the component. I presume to differentiate from the info icon which is very similar visually.

[arthur791004]

Wasn't the idea to build this as an update to the Notice component in Calypso?

I think it would be better to first consider which component we want to replace and plan how to roll out the replacement later. The original idea was to replace the Notice component in Gutenberg. See #103578 (comment).

[jasmussen]

I think it would be better to first consider which component we want to replace and plan how to roll out the replacement later. The original idea was to replace the Notice component in Gutenberg. See #103578 (comment).

Well to clarify: it's all one DS, so even if the ultimate destination is improving the core component directly, for the moment the "upgrade" is being stored in the A8c components package. So to that end, Jay is right. What we want is a single notice: for now that likely has to live in A8c components but with its ultimate destination being WordPress core. What we want to avoid is having multiple components to choose from. Whether that means building this then deprecating the Calypso notice, or just replacing it immediately, I'll leave that to you. But we don't want them both coexisting.

[arthur791004]

Whether that means building this then deprecating the Calypso notice, or just replacing it immediately, I'll leave that to you. But we don't want them both coexisting.

My suggestion would be to deprecate the old component and adopt the new one for all future notices. We can then progressively replace the existing instances of the Calypso notice, as a complete migration will likely require substantial effort 🙂

[jasmussen]

I'll defer to devs (you and others) on the approach there 👍 👍

[jasmussen]

Here's a quick GIF of the state:

At a glance, looks good, will defer to @fcoveram.

[ciampo]

I'll let the rest of @Automattic/team-calypso confirm, but I believe the approach that we have in mind is:

• to review the current implementation and move it to @automattic/components (or whenever we decide to host the A8C DS components);
• to encourage the migration all notices (whether they are a local implementation, or the one from @wordpress/components) to the new implementation.

[arthur791004]

@Automattic/team-calypso Could you help to review this to see if there is any blocker? Thank you 🙂

[taipeicoder]

This looks mergeable on my end. @tyxla @jsnajdr @mirka let us know if there are additional feedback and we'll follow-up.

[arthur791004]

We need this component in Site Settings, and I don’t see any blockers at this point, so let’s move it forward and address any feedback in a follow-up PR 🙂

[ivan-ottinger]

Hey folks 👋🏼🙂 Just a quick question: Do you know if there are any near-term plans to get this Notice component into @wordpress/components? We would like to use it in Studio as well to make the UIs consistent, but it would be great if we could import it directly from WordPress components. Thanks!

[mirka]

@ivan-ottinger There's an internal tracking issue at ARC-692. We don't have immediate plans but it will eventually land in the new @wordpress/ui package. Until then, the current Notice component in @wordpress/components will continue to be the standard Core component.

[ivan-ottinger]

@ivan-ottinger There's an internal tracking issue at ARC-692. We don't have immediate plans but it will eventually land in the new @wordpress/ui package. Until then, the current Notice component in @wordpress/components will continue to be the standard Core component.

Thanks, @mirka. That's helpful.