Change icons from divs to spans

[likimmy]

To address the first issue in this techops, the icons for answers-search-ui should be wrapped in spans, not divs.

[coveralls]

coverage: 62.02%. remained the same
when pulling 0f6f0bb on dev/make-icons-spans
into ff87d8b on hotfix/v1.16.6.

[nmanu1]

just to double check, does all the styling in the test site and acceptance test pages look the same after this change? divs and spans are sometimes styled differently

also, it's probably not a big deal, but we may want to call this change out in the release documentation because any changes to the DOM like this could technically break the styling of an existing implementation if they specifically targeted divs that we're changing to spans

[benmcginnis]

just to double check, does all the styling in the test site and acceptance test pages look the same after this change? divs and spans are sometimes styled differently

also, it's probably not a big deal, but we may want to call this change out in the release documentation because any changes to the DOM like this could technically break the styling of an existing implementation if they specifically targeted divs that we're changing to spans

The Icon class adds a display: inline-block; to the element it's added to so this shouldn't change anything.

[benmcginnis]

How do we do the callout in the release documentation, Nidhi?

[nmanu1]

How do we do the callout in the release documentation, Nidhi?

On our side, this would just mean adding an extra note in our GH release notes explaining possible ramifications of the change. But, it would be up to Baigel if he wants it to be documented more widely, such as in a HH post. The likelihood of this breaking someone's styling is small, but technically possible, so I'm not sure how cautious we want to be about it.

As it is right now, if we release this change in a hotfix, all users on v1.16 will get the change automatically without any action on their part (since most implementations pin to a minor version rather than a patch). This could technically mean that styling may suddenly change for them with no action on their part. If we're worried about this possibility, it may be better to release this in a new minor version so implementations wouldn't get the change without deliberately upgrading. If not, then it's probably fine as is

[nmanu1]

for this techops, would we need to address all instances of divs being used within a button anywhere in answers-search-ui and the HH theme? or did they specifically only care about answers-search-ui icons?

[likimmy]

for this techops, would we need to address all instances of divs being used within a button anywhere in answers-search-ui and the HH theme? or did they specifically only care about answers-search-ui icons?

I'm honestly not sure, I was going off of a code link that was included in the techops that was from answers-search-ui. I'm assuming they only care about the icons from that sdk since they didn't link any other relevant code.

[benmcginnis]

for this techops, would we need to address all instances of divs being used within a button anywhere in answers-search-ui and the HH theme? or did they specifically only care about answers-search-ui icons?

I'm honestly not sure, I was going off of a code link that was included in the techops that was from answers-search-ui. I'm assuming they only care about the icons from that sdk since they didn't link any other relevant code.

I think what's called out in the techops is specifically riven by the the icon component

[likimmy]

to-do item address voiceSearchIcon here

[nmanu1]

to-do item address voiceSearchIcon here

can we update this item to also include looking into any other usages of a div within a button? if this is not valid html, then we'll want to address all of them