Notices: Allow link actions without requiring URLs

[mmtr]

Is your feature request related to a problem? Please describe.

We can define an actions prop for the Notice components in order to display actions related to the notice. This prop is an array of object containing 3 properties: label, url and onClick.

I realized that the action is only styled as a link if the url property is defined, but I think it should be possible to style it as a link even defining only the onClick property.

Observe how the "Learn more" action from the example below is not different from the rest of the notice message.

const actions = [
	{
		label: 'Learn More',
		onClick: () => doSomethingWithoutRedirectToUrl()
	},
];

<Notice status="warning" actions={ actions }>
	To update, check your email and confirm your address.
</Notice>

Describe the solution you'd like

Allow an additional property isLink, so the action can be styled as a link.

const actions = [
	{
		label: 'Learn More',
		isLink: true,
		onClick: () => doSomethingWithoutRedirectToUrl()
	},
];

<Notice status="warning" actions={ actions }>
	To update, check your email and confirm your address.
</Notice>

Describe alternatives you've considered

Since the actions are styled as links if the url property is defined, I tried to use a hack where url is true, so the href attribute of the Button component is not set because true is not a valid value, but the isLink prop is true.

const actions = [
	{
		label: 'Learn More',
		url: true,
		onClick: () => doSomethingWithoutRedirectToUrl()
	},
];

<Notice status="warning" actions={ actions }>
	To update, check your email and confirm your address.
</Notice>

This works, but causes a React warning because true is not a valid value for the href attribute.

Warning: Received `true` for a non-boolean attribute `href`.

[mmtr]

Just realized there is a workaround for avoiding the warning if the url value is # and the onClick callback executes e.preventDefault():

const actions = [
	{
		label: 'Learn More',
		url: '#',
		onClick: e => {
			e.preventDefault();
			doSomethingWithoutRedirectToUrl();
		},
	},
];

<Notice status="warning" actions={ actions }>
	To update, check your email and confirm your address.
</Notice>

Anyway, I still think it'd great to have an isLink option, so this work-around wouldn't be needed.

[afercia]

the url value is #

If the action is doSomethingWithoutRedirectToUrl, i.e. not navigating to another page and "doing something" instead, then the notice should use a <button> element. Let's use button for buttons and links for links. For accessibility reasons, href="#" links should never be used.

[mmtr]

I agree with you @afercia! 🙂

That's why I considered it a work-around and I still would like to see a way of defining buttons styled as links, or alternatively as primary buttons, so they can be differentiated from the notice message.

[afercia]

Looking a bit better, I now understand what you meant. Sorry, I'm a bit slow 😆

So, omitting the url prop and passing the onClick one makes the action thing a button. However, the button is completely unstyled. I'd say this is a bug:

At the very least, those buttons should be automatically styled as default buttons:

Also, I understand the need to style the button as primary, when necessary. So there should be an additional prop to pass class names.

From an accessibility perspective, I'm not so happy that's currently possible to pass both an url prop (so it's a link) and an onClick prop: in this case it should be a button.

An option for your use case could be passing the buttons as children: seems Gutenberg does it for the template validation notice:

gutenberg/packages/editor/src/components/template-validation-notice/index.js

Lines 22 to 28 in 7ea5021

	<Notice className="editor-template-validation-notice" isDismissible={ false } status="warning">
	<p>{ __( 'The content of your post doesn’t match the template assigned to your post type.' ) }</p>
	<div>
	<Button isDefault onClick={ props.resetTemplateValidity }>{ __( 'Keep it as is' ) }</Button>
	<Button onClick={ confirmSynchronization } isPrimary>{ __( 'Reset the template' ) }</Button>
	</div>
	</Notice>

but I understand that's less than ideal and this component should better handle the link / button actions.