feat(applauncher): add support for sections/icons/divider

[mcoker]

fixes #1914

@kybaker I just put images and SVGs of various shapes I found online in the example to show how they will look. There is a 24x24 area to the left of the app launcher link text where the icon will go and the log edge of the icon will stretch to fill the 24px length, and the shorter edge will scale down. And if the logo is less than 24x24, it will just be smaller. All the images will be vertically and horizontally centered.

@jgiardino I changed the title <h2>s to <h1>s, inline with what we're doing in the dropdown component, since you can establish a new heading hierarchy in <section> elements. And added the scree-reader text.

[patternfly-build]

Deploy preview for pf-next ready!

Built with commit 9bf6fdd

https://deploy-preview-1916--pf-next.netlify.com

[jgiardino]

👍 This looks great!

I really like how you combined the icon with the screen reader text, since they are synonymous. Looking at this html example below, this seems like a pattern that should be reusable in other places. A similar case came up with my product team today where they want to include a link to the product documentation in a dropdown menu in the masthead. Would it be possible to make this chunk of the menu item a reusable component? Or is that just something we should capture as part of #1429?

<span class="pf-c-app-launcher__menu-item-external-icon">
  <i class="fas fa-external-link-alt" aria-hidden="true"></i>
  <span class="pf-screen-reader">(opens new window)</span>
</span>

One change that I recommend is to not set opacity on .pf-c-app-launcher__menu-item-external-icon and instead set it on just the icon, to avoid any potential issues with opacity affecting screen reader perception of that text.

[jgiardino]

See my comment above. The opacity should not be applied to the screen-reader-only text.

[mcoker]

@jgiardino thanks, updated so that we don't have a wrapper for the external link icon, it seems unnecessary. Also updated the documentation. Should aria-hidden="true" be applied to the icon on the left of the list item, too?

[mcoker]

@jgiardino

Would it be possible to make this chunk of the menu item a reusable component?

What would be the benefit of it being a component? I'm also curious how we would position it as a component - what might it be named? I also changed the structure with an update (removed the wrapper and now just have the icon + screen-reader text) and, assuming that works, curious if that impacts this potentially becoming a component?

[jgiardino]

Should aria-hidden="true" be applied to the icon on the left of the list item, too?

Since you are using <img> elements for these icons, AND since these images are redundant (i.e. the text that follows is considered synonymous with these icons), then having alt="" like you have it here is the correct way to tell assistive technology that this image provides no extra meaning and to ignore it.

[jgiardino]

What would be the benefit of it being a component?

I think this just gets back to the need to have menu items and toggles as separate components so that we can build these things just once (e.g. this would be a variation of the MenuItem component). Should I just add this as something to #1429 for now?

This is definitely something that could be useful in the Dropdown menu, only because this exact use case came up in a product planning meeting yesterday. :-)

I also changed the structure with an update (removed the wrapper and now just have the icon + screen-reader text) and, assuming that works, curious if that impacts this potentially becoming a component?

I think this works. I think if it's just a variation of a future menu item component, this would be fine.

[mcoker]

Should I just add this as something to #1429 for now?

@jgiardino that sounds good to me, thanks!

[jgiardino]

I have a question for design (@mcarrano, @kybaker)...

Is the use case for this component that it will always have links?

A question came up off thread about whether the app launcher should always be wrapped in a <nav> element. My thinking is that if the consumer needs a similar interaction pattern that provides a menu of action buttons instead of navigational links, then that's what the Dropdown should be used for. Does that match your expectation?

Also, in other components where we use the <nav> element, we try to be prescriptive in how we define the labels for those regions. This is really important when screen reader users are navigating by region and there are multiple navigation regions on the page. Some of these labels we decided on are "Global" for primary nav, "Local" for tertiary nav, and "Breadcrumb" for breadcrumbs (e.g. VO announces this region as "Breadcrumb navigation").

If the app launcher is wrapped in a navigation region, what would that label be? Would "Application" match our use case for this component?

@mcoker If we do decide that all app launchers should use <nav> and that all aria-labels should be more specific to our use case for this component, should that be captured in a separate issue (since that would require updating all examples and not just this new one)?

[mcoker]

@mcoker If we do decide that all app launchers should use

and that all aria-labels should be more specific to our use case for this component, should that be captured in a separate issue (since that would require updating all examples and not just this new one)?

@jgiardino I'm fine with making that update in this PR if you would be OK with verifying that changeset.

[mattnolting]

Looks fantastic, really top-notch. Just caught the three css vars.

[mcarrano]

My thinking is that if the consumer needs a similar interaction pattern that provides a menu of action buttons instead of navigational links, then that's what the Dropdown should be used for. Does that match your expectation?

Yes, this matches my expectations @jgiardino . Regarding the naming, I think "Application" could be ambiguous. Maybe "Application lancher" or just "launcher?"

[jgiardino]

Regarding the naming, I think "Application" could be ambiguous. Maybe "Application lancher" or just "launcher?"

I was also wondering the same thing, and thinking that "Application launcher" might be better.

"Launcher" alone seems ambiguous to me, too.

And something to keep in mind is that this label gets combined with "navigation", so for the user it would get announced as "Application launcher navigation".

[mcoker]

@jgiardino updated the examples and documentation.

[mcoker]

@mattnolting thanks, copied and pasted that code from the dropdown component, so committed your suggestions and updated the dropdown component if you want to take another look.

[mattnolting]

Perfect 👍

[jgiardino]

[patternfly-build]

🎉 This PR is included in version 2.13.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀