Unknown styling is injected into the document when using @azure/communication-react: v1.9.0-beta.1

musale commented 8 months ago

Describe the bug; what happened?

When using the MessageThread component, unknown CSS is injected which will change the UI styling used.

What are the steps to reproduce the issue?

Using the sample app in https://github.com/microsoftgraph/microsoft-graph-toolkit/tree/main/samples/react-contoso, update the @azure/communication-react package to 1.9.0-beta-1 or any version above it.
Copy the content in .env.sample to a .env file.
Run yarn && yarn build && yarn watch:react.
App loads. Login to view the Chat tab. Click on it.
Re-load the page and see the UI load and then change.

What behavior did you expect?

The UI to load without being changed.

If applicable, provide screenshots:

Using v1.8.0 and below, the UI renders correctly:

Upgrading to any version from v1.9.0-beta.1 or higher you get

In what environment did you see the issue?

@azure/communication-react : 1.8.0, ^1.9.0-beta.1
@azure/communication-chat : 1.4.0
OS & Device: Windows on PC
Browser: Microsoft Edge
Browser Version: 121.0.2277.71 Is there any additional information?

palatter commented 8 months ago

@musale Thank you for reporting this issue. I will take a look and give you an updates soon.

palatter commented 8 months ago

The main branch doesn't have any dependencies for azure/communication? Is there a specific branch you are working out of?

musale commented 8 months ago

@musale Thank you for reporting this issue. I will take a look and give you an updates soon.

Yes. Use this next/mgt-chat branch https://github.com/microsoftgraph/microsoft-graph-toolkit/tree/next/mgt-chat

palatter commented 8 months ago

I checked out the above branch and was able to run the sample.

I am not able to login in though. Are there some permissions required?

I have the .env file set as stated in readme

palatter commented 8 months ago

Is there a specific test tenant that I need to use?

musale commented 8 months ago

Is there a specific test tenant that I need to use?

No specific test tenant is required. A normal test tenant should suffice. Are you still experiencing the login issues?

emlynmac commented 8 months ago

Hey @musale, I was able to run in and reproduce an issue with the visual styling of the dashboard app.. The affected components seem to be the side menu and the chat area container, as you can see with the video.

https://github.com/Azure/communication-ui-library/assets/3941071/b232cd33-8739-4b48-bc15-e1899412e9d2

It also corrects itself on opening / closing the side panel menu and after that there's no issue with switching between threads.

One of the key differences between 1.9.0-beta.1 and 1.8.0-beta.1 is that we have fully removed the fluent-northstar package from the ui library. During this, we found some issues with global CSS that was provided by northstar. You can see the changes here:

https://github.com/Azure/communication-ui-library/blob/main/packages/react-components/src/theming/FluentThemeProvider.tsx#L29

I was able to remove the @fluentui/react-northstar depenedency from mgt-chat without issue.

It looks like var(--spacingHorizontalMNudge) is causing problems, on the .f1ng84yb class.

The variable is undefined in the case of the visuals failing. This issue seems to be across all the CSS variables, some more examples:

.fhovq9v {
    background-color: var(--colorSubtleBackground);
}
f1i3iumi {
    line-height: var(--lineHeightBase300);
}
.fk6fouc {
    font-family: var(--fontFamilyBase);
}

The values are all provided by .fui-FluentProvider1 It looks like the fluent theme variables are not kept when switching threads.

musale commented 8 months ago

@emlynmac I removed the dependency @fluentui/react-northstar that we used the List and ListItem from. I'm running the latest release of ACS without issues. I have shifted to the Migration shims for now. Thank you for your guidance on this, I will close it as it is not a direct issue in acs.

musale commented 8 months ago

As a note @emlynmac will you need to mention that removal of @fluentui/react-northstar might affect other parts of the application using it alongside acs?

emlynmac commented 8 months ago

As a note @emlynmac will you need to mention that removal of @fluentui/react-northstar might affect other parts of the application using it alongside acs?

I think we need to establish what the issue is exactly. So far, we've not degradation of the tests / samples like this.

Azure / communication-ui-library

Unknown styling is injected into the document when using @azure/communication-react: v1.9.0-beta.1 #4058