Input: contentBefore and contentAfter don't work with bundleIcons

[ling1726]

Repro: https://codesandbox.io/s/mystifying-austin-02jpd4?file=/example.tsx

The specificity of the content styles in Input are more specific than the styles applied in bundled icons, the result is that both filled and regular icons are always visible

fluentui/packages/react-components/react-input/src/components/Input/useInputStyles.ts

Lines 215 to 235 in 66bf89f

	const useContentStyles = makeStyles({
	base: {
	boxSizing: 'border-box',
	color: tokens.colorNeutralForeground3, // "icon color" in design spec
	// special case styling for icons (most common case) to ensure they're centered vertically
	'> svg': { display: 'block' },
	},
	disabled: {
	color: tokens.colorNeutralForegroundDisabled,
	},
	// Ensure resizable icons show up with the proper font size
	small: {
	'> svg': { fontSize: '16px' },
	},
	medium: {
	'> svg': { fontSize: '20px' },
	},
	large: {
	'> svg': { fontSize: '24px' },
	},
	});

[nandin-borjigin]

I don't see a value for bundleIcons to always render both of the icon variants and control their display rule.

An icon being filled or regular is mostly a static decision and doesn't seem to change at runtime. So, a logic like props.fill ? <FilledVariant /> : <RegularVariant /> would be much more robust.

It is correct for consumers of bundleIcon to expect bundleIcon only renders one element and they may do some other modifications like .parent:nth-child(0) { /* some rule */ }. The fact that bundleIcon renders two elements actually breaks such assumption.

[ling1726]

An icon being filled or regular is mostly a static decision and doesn't seem to change at runtime.

This assumption isn't necessarily correct, quite often designs call for 'hover' icons that are filled on hover

[nandin-borjigin]

@ling1726 , thanks for clarifying, I didn't know that.

But still, switching between display: none and display: inline still affects the render tree (as opposed to DOM tree) and causes a reflow, so I presume it doesn't bring too much value in terms of perf compared with filled ? <FilledVariant /> : <RegularVariant />.

However, doing so brings inconsistency between the actual behavior of the wrapper component and its expected semantic (User expects one element out of it but it actually yields two)

[ling1726]

@nandin-borjigin, implementing this with javascript isn't a great idea since you would need to use event handlers like onMouseEnter and onMouseLeave to make this possible and might scripting perf concerns + being more flaky since it needs both event handlers to work correctly. the CSS based solution means that they can be styled with the :hover pseudo selector.