Grafana UI: Add description to Menu component

[axelavargas]

What is this feature?
Adds to the Menu component a new description option (majority of this code was taken from this PR by @gelicia

ItemMenuDescription.mp4

Why do we need this feature?

Users can benefit from more clarity in some complex menus.

Who is this feature for?

everyone

Which issue(s) does this PR fix?:

Fixes #

Special notes for your reviewer:

Please check that:

• It works as expected from a user's perspective.
• If this is a pre-GA feature, it is behind a feature toggle.
• The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

[joshhunt]

I know it was in the other PR this came from, but we don't think the description should be italic. Otherwise, this looks good! 👍

[axelavargas]

oh, thanks for the heads up 🙌🏾 . I did the changes :)

[ashharrison90]

@axelavargas wanted to see if we could avoid adding the wrapping div, but didn't want to lump you with that so did some refactoring here: https://github.com/grafana/grafana/compare/axelav/add-description-to-menu...ash/menu-refactor?expand=1

i think this is the exact same behaviour you want just with a smaller amount of changes (and also fixes some weird things like negative margin on the icons 😬). if you're happy you can cherry-pick the commit 4afca39 onto your branch and it should cut down the diff a fair bit 👍

[axelavargas]

@axelavargas wanted to see if we could avoid adding the wrapping div, but didn't want to lump you with that so did some refactoring here: axelav?expand=1 (compare)

i think this is the exact same behaviour you want just with a smaller amount of changes (and also fixes some weird things like negative margin on the icons 😬). if you're happy you can cherry-pick the commit 4afca39 onto your branch and it should cut down the diff a fair bit 👍

Thank you Mr Harrison 😎, the only thing with that approach is related to a11y, sometimes ItemElement can be a button or an a, and the Stack component is a div, this will make a11y fail :(

[ashharrison90]

@axelavargas wanted to see if we could avoid adding the wrapping div, but didn't want to lump you with that so did some refactoring here: axelav?expand=1 (compare)
i think this is the exact same behaviour you want just with a smaller amount of changes (and also fixes some weird things like negative margin on the icons 😬). if you're happy you can cherry-pick the commit 4afca39 onto your branch and it should cut down the diff a fair bit 👍

Thank you Mr Harrison 😎, the only thing with that approach is related to a11y, sometimes ItemElement can be a button or an a, and the Stack component is a div, this will make a11y fail :(

oh, does that fail a11y? 🤔 i thought you could nest a div inside 😅

if not, could we use a <span> instead of <Stack> with an appropriate class of something like:

css({
  display: 'flex',
  flexDirection: 'row',
  justifyContent: 'flex-start',
  alignItems: 'center',
})

[axelavargas]

....
oh, does that fail a11y? 🤔 i thought you could nest a div inside 😅

if not, could we use a <span> instead of <Stack> with an appropriate class of something like:

css({
  display: 'flex',
  flexDirection: 'row',
  justifyContent: 'flex-start',
  alignItems: 'center',
})

Let me confirm again about a11y, now I am hesitating 😅

[axelavargas]

@ashharrison90 alright, automation a11y tooling says it should be ok 😄 , I was mistaken. I will use the refactor, thanks a lot for helping with this 🥇

[ashharrison90]

lgtm 👏

[joshhunt]

lgtm. nice team work 🙏