Emails: Fix mailbox list background and position

[m1r0]

Resolves #101651

Proposed Changes

• Fixes the missing background color on the mailbox list and adds a bit of spacing between the sections.

Why are these changes being made?

• This fixes a UI bug.

Testing Instructions

• Have a domain with an email subscription.
• Navigate to /email/:domain/manage/:site.
• Make sure the "Mailboxes" section looks similar to the design: ePfz53j71KKDLWtuSm567A-fi-4962_13952
• Navigate to /overview/site-domain/email/:site/:domain and /domains/manage/all/email/:site/:domain and make sure the "Mailboxes" section looks good.
• Check on mobile.

Screenshots

/email/:domain/manage/:site:

/overview/site-domain/email/:site/:domain:

/domains/manage/all/email/:site/:domain:

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-137998

Jetpack Cloud live (direct link)

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

Automattic for Agencies live (direct link)

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

[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.

[fredrikekelund]

Thanks for the quick turnaround, @m1r0 👍

This PR fixes the changes, but I would argue we should apply the styling to the existing component structure instead of adding a VerticalNav component here (given that the children aren't VerticalNavItems)

[fredrikekelund]

Suggested change

	<div className="vertical-nav">
	<SectionHeader
	isPlaceholder={ isPlaceholder }
	label={
	<>
	{ !! showIcon && domain && <HeaderIcon /> }
	{ getListHeaderTextForAccountType() }
	</>
	}
	>
	{ addMailboxPath && (
	// eslint-disable-next-line @typescript-eslint/ban-ts-comment
	// @ts-expect-error
	<Button href={ addMailboxPath } variant="link" disabled={ !! disableActions }>
	{ translate( 'Add mailbox' ) }
	</Button>
	) }
	{ showContextMenu && (
	<ContextMenu
	domain={ domain }
	className="email-plan-mailboxes-list__mailbox-context-menu"
	/>
	) }
	</SectionHeader>
	{ children }
	</div>
	<SectionHeader
	isPlaceholder={ isPlaceholder }
	label={
	<>
	{ !! showIcon && domain && <HeaderIcon /> }
	{ getListHeaderTextForAccountType() }
	</>
	}
	>
	{ addMailboxPath && (
	// eslint-disable-next-line @typescript-eslint/ban-ts-comment
	<Button href={ addMailboxPath } variant="link" disabled={ !! disableActions }>
	{ translate( 'Add mailbox' ) }
	</Button>
	) }
	{ showContextMenu && (
	<ContextMenu
	domain={ domain }
	className="email-plan-mailboxes-list__mailbox-context-menu"
	/>
	) }
	</SectionHeader>
	{ children }

It looks like the only purpose vertical-nav is to add a margin-top: 15px rule in the single-site context. Applying that style to email-plan-mailboxes-list__mailbox-list directly seems sensible, since we're not using VerticalNavItem here.

Side note: the // @ts-expect-error looks to be superfluous unless I'm missing something.

[m1r0]

Thanks for the quick review, Fredrik. All makes sense! I've moved the styling to the component and cleaned up the vertical-nav (not sure why I went with the div instead of the component). Can you give it another quick look? There should be no visual changes.

Regarding the @ts-expect-error, I've tried removing it but that unveiled a strange TypeScript error related to the href. 🤷 I wasn't able to find a reasonable explanation, so I'm leaving it for now.

[fredrikekelund]

Looking good and tests well 👍 Thanks for the update and sorry for my delayed second review, @m1r0