microsoft / fast

The adaptive interface system for modern web experiences.
https://www.fast.design
Other
9.23k stars 590 forks source link

Unable to set disabled attribute on FluentUI Checkbox in ReactJS. #6266

Closed apanwar22i closed 2 years ago

apanwar22i commented 2 years ago

Hi, I'm facing an issue with Fluent Checkbox. When I try to set "disabled" attribute on the control, the VS Code throw an error message:

Type '{ children: string; disabled: true; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<Component<ReactWrapperProps<FoundationElement, unknown>, {}, any>> & Readonly<...>'. Property 'disabled' does not exist on type 'IntrinsicAttributes & IntrinsicClassAttributes<Component<ReactWrapperProps<FoundationElement, unknown>, {}, any>> & Readonly<...>'.

We are using FluentUI Component inside ReactJS by using wrapping method. Strange thing is that, "disabled" property working on Fluent button. Please take a look to the screenshot:

image

NPM Packages on package.json:

{ "@fluentui/web-components": "^2.5.3", "@microsoft/fast-element": "^1.10.3", "@microsoft/fast-foundation": "^2.46.10", "@microsoft/fast-react-wrapper": "^0.3.11", "@microsoft/mgt-element": "^2.5.2", "@microsoft/mgt-msal2-provider": "^2.5.2", "@microsoft/mgt-react": "^2.5.2", "@microsoft/microsoft-graph-client": "^3.0.2", "@microsoft/microsoft-graph-types": "^2.22.0", "@microsoft/teams-js": "^1.6.0", "@reduxjs/toolkit": "^1.8.3", "@testing-library/jest-dom": "^5.16.4", "@testing-library/react": "^13.3.0", "@testing-library/user-event": "^13.5.0", "@types/jest": "^27.5.2", "@types/react": "^18.0.15", "@types/react-dom": "^18.0.6", "@types/react-redux": "^7.1.24", "@types/redux": "^3.6.0", "@types/underscore": "^1.11.4", "axios": "^0.27.2", "bootstrap": "^5.1.3", "cross-env": "^7.0.3", "moment": "^2.29.4", "moment-timezone": "^0.5.34", "newTeamsjs": "npm:@microsoft/teams-js@^2.1.0", "react": "^18.2.0", "react-dom": "^18.2.0", "react-redux": "^8.0.2", "react-router-dom": "^6.3.0", "react-scripts": "5.0.1", "reactstrap": "^9.1.1", "redux": "^4.2.0", "redux-persist": "^6.0.0", "typescript": "^4.7.4", "underscore": "^1.13.4", "web-vitals": "^2.1.4" }

Code snippet:

import * as React from 'react';
import { fluentCheckbox, fluentButton, provideFluentDesignSystem } from '@fluentui/web-components';
import { provideReactWrapper } from '@microsoft/fast-react-wrapper';

const { wrap } = provideReactWrapper(React, provideFluentDesignSystem());

const FluentCheckbox = wrap(fluentCheckbox());
const FluentButton = wrap(fluentButton());

export const MyComponent: React.FC<any> = (props: any) => {
    return (
        <div>
        <FluentCheckbox disabled>Check me</FluentCheckbox>
        <FluentButton disabled>Click me</FluentButton>
        </div>
    )
}

I also try to explore the FluentUI files inside node_modules for more details. But i couldn't found any "disabled" property inside the Fluent Checkbox code, but other properties are there.

Please take a look on this issue, hope I'm not missing anything.

Thanks.

EisenbergEffect commented 2 years ago

Looks like disabled may indeed be a missing feature. @chrisdholt Can you confirm?

chrisdholt commented 2 years ago

Looks like disabled may indeed be a missing feature. @chrisdholt Can you confirm?

This is inherited from FormAssociated, it exists on the class, is referenced in Checkbox and is able to be set: https://github.com/microsoft/fast/blob/21f3145620c12842a88314ea4f46899626fb6960/packages/web-components/fast-foundation/src/form-associated/form-associated.ts#L378

Stackblitz illustrating non-react scenario: https://stackblitz.com/edit/fluent-select-example-rajvmo?file=index.html

EisenbergEffect commented 2 years ago

Oops. Looks like I missed that when I was scanning the code. Hmm. Odd that TypeScript says it isn't there on the React Wrapper. That should pick up all public properties. I'll need to look into this separately. I'll add this to my list. @apanwar22i I'll add this to my list but not sure if I'll be able to get to it this week.

KingOfTac commented 2 years ago

I just tested using react@18.2.0 and fluentui@2.5.3 with fast-react-wrapper@0.3.11 and everything seems to be working fine. https://stackblitz.com/edit/react-ts-wr3hj1?file=App.tsx

I'm wondering if it is issues caused by microsoft/mgt using an older version of fluentui which may be using an older version of fast-element and foundation than what @apanwar22i has specified in the package.json

EisenbergEffect commented 2 years ago

That could certainly cause the issue, yep.

apanwar22i commented 2 years ago

@KingOfTac @EisenbergEffect So this is happening because of MGT? I checked your code snippet on stackblitz, we are using same version.

EisenbergEffect commented 2 years ago

It could be a conflict between the version of the components that MGT is bundled with. I'm reaching out to the MGT team to see if they have any insight on this. Thanks for your patience.

apanwar22i commented 2 years ago

@EisenbergEffect Is there any alternate solution/workaround for now?

EisenbergEffect commented 2 years ago

@apanwar22i Check to make sure you don't have two versions of the components in your solution. Have a look at your lock file. If you find two versions of fast or Fluent then try to manually resolve them all to the latest versions.

I don't know a lot about how MGT is put together. I've reached out to their team to try to see if we can get you a solution. But I'd start by checking for duplicates first, just in case there's a simple solution through deduping.

apanwar22i commented 2 years ago

@EisenbergEffect Can you please review these npm packages that we are using on our React application. Is all fast packages are mandatory to use?

"@fluentui/web-components": "^2.5.3",
"@microsoft/fast-element": "^1.10.3",
"@microsoft/fast-foundation": "^2.46.10",
"@microsoft/fast-react-wrapper": "^0.3.11",
"@microsoft/mgt-element": "^2.5.2",
"@microsoft/mgt-msal2-provider": "^2.5.2",
"@microsoft/mgt-react": "^2.5.2",
"@microsoft/microsoft-graph-client": "^3.0.2",
"@microsoft/microsoft-graph-types": "^2.22.0",
"@microsoft/teams-js": "^1.6.0",
apanwar22i commented 2 years ago

@EisenbergEffect @KingOfTac

We just found out a Fluent UI link for React https://developer.microsoft.com/en-us/fluentui#/

The version mentioned there is v9. Name of the npm package is:

@fluentui/react
@fluentui/react-northstar

But these packages were not mentioned on the FluentUI documentation link: https://docs.microsoft.com/en-us/fluent-ui/web-components/integrations/react

@fluentui/web-components 
@microsoft/fast-foundation 
@microsoft/fast-element 
@microsoft/fast-react-wrapper

Can you please help us out if we arent using any incorrect package.

Thanks

EisenbergEffect commented 2 years ago

There are two implementations of Fluent UI for Web. One implementation is done with Web Components and the other implementation is done with React. The Microsoft Graph Toolkit components are built as Web Components and thus leverage the Fluent UI Web Components (not the React implementations) internally.

Web Components work with any library or framework because they are the web standard for components. However, each framework tends to have a bit of configuration that's needed based on the framework's quirks. To explain that configuration, we have a set of integration docs showing how to use Web Components with each major framework. e.g. React, Angular, Vue, etc.

The documentation on docs.microsoft.com that you've linked above covers our Fluent UI Web Component implementation, not the React implementation. The integration docs cover how to integrate the Web Components with an existing React application.

Here's an explanation of the packages related to the Fluent UI Web Components:

You have a couple of options for how to build your application:

  1. Build everything with Web Components. They are smaller, faster, and more more efficient than React Components. In this case you would use fast-element to build your application in combination with @fluent-ui/web-components.
  2. Build your application entirely with React. You would use React to build your own components and then use @fluentui/react for the Fluent implementation.
  3. Build your application with React but leverage a design system built with Web Components. In this case you would use the @fluent-ui/web-components inside of your React application as your core component library but continue to build your own components with React.

However, if you need to integrate with the Microsoft Graph Toolkit, you need to make special consideration for that, since there's only a Web Component implementation available. So, if you are building your application with React, then you will likely want to use the MGT React Wrapped components. However, as stated above, MGT already uses @fluent-ui/web-components internally because MGT is fundamentally a Web Component library, not a React library. So, if you use MGT, you probably want to also use @fluent-ui/web-components, otherwise you will end up with two different implementations of the same components in your app.

If you are building from scratch, without any pre-existing React code, then the easiest path is to go all Web Components. If you have an existing React application but don't need to use MGT, then you may find it easier to use the pure React components, though they are larger and slower than the Web Component equivalents. So, if you are concerned about performance, you may instead want to use React-wrapped Web Components with your existing React application. If you need to use MGT, then you are going to be using Web Components regardless, because there is only a Web Component implementation (with a React wrapper). And if you are going to be using MGT, then it's also probably better to also use the Fluent UI Web Components.

In other words, you are probably using the right combination of technologies, assuming you have a pre-existing React codebase and need to use MGT. However, if this is a greenfield project and you have no pre-existing React code, then you will likely find it simpler to use Web Components for everything, which will also reduce your bundle size and improve the performance of your experience.

If all of this sounds confusing, I agree, and I sincerely apologize. I know this is difficult for customers right now. I am working internally across product groups, talking to many VPs and other members of leadership to try to drive for a simpler technical solution and clearer messaging, along with better support resources to help people through issues. I really appreciate how patient you and your team have been with this process.

apanwar22i commented 2 years ago

Thank you @EisenbergEffect for detailed information, appreciate your efforts. Coming back to the subjected issue, We will wait for your response. Please let us know once you get the solution from MGT team. Or do you suggest we need to raise separate issue for MGT team?

EisenbergEffect commented 2 years ago

@apanwar22i No need to bother the MGT team. I am coordinating with them internally on your behalf. We've got a repro from MGT now and they've handed it off to me for the next phase of investigation. I'm looking into it now.

EisenbergEffect commented 2 years ago

Ok, I believe I've uncovered the issue. There appears to be a type issue where the Checkbox type information is not properly flowing through the wrapper. We can fix this in our next major version (Actually I think we already fixed it. I'll double check.)

Fortunately, the workaround is pretty straight forward. You can cast the output of the wrapper to a more specific type. You will only need to do this if you see missing properties. Most of the components work correctly but apparently there are a few that have this issue. Here's some code that shows how to handle it:

import * as React from 'react';
import {
    fluentButton,
    fluentCard,
    fluentCheckbox,
    fluentDivider,
    provideFluentDesignSystem
} from '@fluentui/web-components';
import { provideReactWrapper, ReactWrapper } from '@microsoft/fast-react-wrapper';
import { Checkbox } from '@microsoft/fast-foundation'; // get the type from foundation

const { wrap } = provideReactWrapper(React, provideFluentDesignSystem());

export const FluentButton = wrap(fluentButton());
export const FluentCard = wrap(fluentCard());
export const FluentCheckbox = wrap(fluentCheckbox()) as ReactWrapper<Checkbox, {}>; //cast!
export const FluentDivider = wrap(fluentDivider());

const jsx = <FluentCheckbox disabled></FluentCheckbox>;
EisenbergEffect commented 2 years ago

@apanwar22i Let me know if this gets you going again. Thanks for your patience.

apanwar22i commented 2 years ago

Thanks @EisenbergEffect for the solution. The workaround you were suggested it work and we are able to use "disabled" attribute inside the FluentCheckbox component. Thank you so much for your support. We'll let you know if we may need further assistance .

apanwar22i commented 2 years ago

@EisenbergEffect @KingOfTac Got an another issue with the checkbox events. Can you please take a look this issue: https://github.com/microsoft/fast/issues/6313