fluentui icon indicating copy to clipboard operation
fluentui copied to clipboard

[Bug]: MessageBar icon props does not allow for width, height, line-height, or color overrides

Open tronguye opened this issue 3 years ago • 3 comments
trafficstars

Library

React / v8 (@fluentui/react)

System Info

System:
    OS: Windows 10 10.0.19044
    CPU: (12) x64 Intel(R) Xeon(R) W-2135 CPU @ 3.70GHz      
    Memory: 22.86 GB / 63.59 GB
  Browsers:
    Edge: Spartan (44.19041.1266.0), Chromium (102.0.1245.44)
    Internet Explorer: 11.0.19041.1566

Are you reporting Accessibility issue?

No response

Reproduction

https://codepen.io/tronguye/pen/JjLjpaL

Bug Description

When passing font size, height, width, line height, or color to dismissIconProps.styles:

Actual Behavior

  1. Icon is black, with 10px font size, width, height, and line height

Expected Behavior

  1. The styles passed to the icon styles prop is preferred

Workaround: add !important to take higher precedence over the underlying icon button selector. Not ideal, however

Logs

No response

Requested priority

Normal

Products/sites affected

No response

Are you willing to submit a PR to fix?

no

Validations

  • [X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
  • [X] The provided reproduction is a minimal reproducible example of the bug.

tronguye avatar Jun 29 '22 00:06 tronguye

@tronguye - Thanks for filing this issue with us. To set expectations the developers will review this issue once capacity allows.

@layershifter - Would you be able to confirm if this is a regression, or if this behavior is an issue? Thanks

gouttierre avatar Aug 08 '22 10:08 gouttierre

@tronguye this is how styling done in this component. To override this styles without !important you can do following:

function App() {
  return (
    <MessageBar
      dismissIconProps={{
        iconName: "Add"
      }}
      styles={{
        dismissal: {
          "& .ms-Button-icon": {
            fontSize: "20px",
            height: "20px",
            width: "20px",
            color: "blue"
          }
        }
      }}
    />
  );
}

https://codesandbox.io/s/sad-wright-0xjsfl?file=/src/App.tsx:433-882

layershifter avatar Aug 09 '22 11:08 layershifter

Thanks Oleksander!

Ideally the consumers of this component don't need to know about the underlying class .ms-Button-icon which could be a brittle class name not guaranteed by the public/semver contract of the package. I'm a little surprised the styling of the dismiss icon does not live in dismissIconProps

It may be best for that class name / property to be documented at least!

tronguye avatar Aug 09 '22 17:08 tronguye

@tronguye grad that have figure out the problem.

Ideally the consumers of this component don't need to know about the underlying class .ms-Button-icon which could be a brittle class name not guaranteed by the public/semver contract of the package. I'm a little surprised the styling of the dismiss icon does not live in dismissIconProps

Unfortunately I don't know the motivation behind that styling, but there is probably a reason. We also can't do much with it as changing selectors or removing them will be a breaking change.

I briefly checked other issues/documentation and I don't see this being documented anywhere (please correct me if a wrong). While it might be unobvious styles can be inspected in Chrome/Edge Devtools to code:

https://github.com/microsoft/fluentui/blob/0fa48e25fd9b173a8574c3ad3421979e627a145d/packages/react/src/components/MessageBar/MessageBar.styles.ts#L108


FYI: As it's not a bug I am going to close the issue.

layershifter avatar Aug 12 '22 09:08 layershifter