SharePoint / sp-dev-docs

SharePoint & Viva Connections Developer Documentation
https://docs.microsoft.com/en-us/sharepoint/dev/
Creative Commons Attribution 4.0 International
1.25k stars 1.01k forks source link

New custom dialog boxes have a max width of 600px #9929

Open svkcode opened 1 month ago

svkcode commented 1 month ago

Target SharePoint environment

SharePoint Online

What SharePoint development model, framework, SDK or API is this about?

πŸ’₯ SharePoint Framework

Developer environment

Windows

What browser(s) / client(s) have you tested

Additional environment details

Describe the bug / error

A recent update to dialog boxes (rounded corners) in SharePoint is causing dialogs created with the sp-dialog library to have a max width of 600px. Content that is larger than 600px that used to fit is now overflowing.

image

Steps to reproduce

  1. Create a SPFx command set extension that displays a custom dialog box using the sp-dialog library with content larger than 600px
  2. Run the extension on a SharePoint list and execute the command to display the custom dialog

Expected behavior

Custom dialog boxes should expand to fit their content

AbshireMI commented 1 month ago

Can confirm this doesn't just affect new dialogs or apps - this is affecting multiple existing applications in our environment.

Here is an example of a dialog as we expect it to be displayed (some elements redacted for confidentiality): image

And here is an example of how this same dialog now renders: image

We have narrowed the issue to the r1svjbtt class, as shown in @svkcode's original post.

There are more issues than the max-width, in particular the padding is also an issue, as can be seen from the example screenshots.

This same issue occurred on 2024-08-26 and lasted for most of the day (UK time, BST).

We do not currently have a workaround for this issue, which is causing several inhouse applications to be unusable.

AbshireMI commented 1 month ago

OK, we now have a workaround, but it feels very much more like a MacGuyver than a solution ... especially given that we need to apply this to all apps, and then the need to undo the change once the underlying issue is resolved.

In our FunctionDialog.module.scss we have added the following:

.r1svjbttOverride {
  padding: unset !important;
  max-width: 65vw !important;
}

The exact value for max-width will depend on the use-case for the dialog, in some cases we're going as high as 90vw others need less. Using vw units allows at least some scaling with viewport size, while not ideal it's better than many of the other options.

In the FunctionDialog.tsx we then add:

import styles from './FunctionDialog.module.scss';

And in the render() method in FunctionDialog.tsx we add:

document.getElementsByClassName('fui-DialogSurface')[0].classList.toggle(styles.r1svjbttOverride, true);

No changes are required in FunctionDialogContent.tsx.

This does not restore the previous dynamic sizing functionality, and simply removing the padding as we do here causes the top highlight and corner radii of the dialog to render incorrectly, but this at least allows us to redeploy all of our apps into a functional state until this issue can be triaged and resolved.

jcoolsen commented 1 month ago

For me, I had a slightly different approach, using a ref to walk up the dom tree and applying max-width: fit-content. I guess it all depends on your circumstances. For now I'm ignoring the extra padding as "standard" SharePoint.

AbshireMI commented 1 month ago

For me, I had a slightly different approach, using a ref to walk up the dom tree and applying max-width: fit-content. I guess it all depends on your circumstances. For now I'm ignoring the extra padding as "standard" SharePoint.

Unfortunately fit-content, at least as an override to the faulting class, doesn't quite work in our use-case - the resulting dialog is still much narrower than expected - but that would certainly work well in most cases too.

JurajSlavik commented 1 month ago

Confirming the same behaviour for existing applications. To ensure thei functionality, until this will be fixed oficially , we have used setting of min-width of the div with dialog role in dialog styles:

@media only screen and (min-width: 660px) {
  :global {
    div[role="dialog"] {
      display: inline-table;
      min-width: 660px;
    }
  }
}

@media only screen and (min-width: 1024px) {
  :global {
    div[role="dialog"] {
      display: inline-table;
      min-width: 1024px;
    }
  }
}
christophepeerens commented 1 month ago

Since there's no β€œofficial” reaction, is this also considered a β€œbug”, or will the style stay that way? Can someone from MS give us some feedback?

ymyqwe commented 1 month ago

Sorry for causing the trouble. This is caused by Fluent UI v9 upgrade as the default max-width was set as 600px here https://react.fluentui.dev/?path=/docs/components-dialog--docs. And the issue should be mitigated now. We will have a fix for this to move forward.

AbshireMI commented 1 month ago

Sorry for causing the trouble. This is caused by Fluent UI v9 upgrade as the default max-width was set as 600px here https://react.fluentui.dev/?path=/docs/components-dialog--docs. And the issue should be mitigated now. We will have a fix for this to move forward.

I'm not sure how you think this issue is mitigated - on removing our workaround (because your mitigation caused this to break every dialog, preventing it from even opening), we find that the dialogs now size to about 688px instead of 600px.

Can you evidence your testing process for this most recent change? What internal and community testing was carried out before pushing the change to production?

We're now back in a position where applications which have been stable for years are now non-functional, and we're once again working to implement mitigations and workarounds to changes pushed to production without notice.

ymyqwe commented 1 month ago

Sorry for causing the trouble. This is caused by Fluent UI v9 upgrade as the default max-width was set as 600px here https://react.fluentui.dev/?path=/docs/components-dialog--docs. And the issue should be mitigated now. We will have a fix for this to move forward.

I'm not sure how you think this issue is mitigated - on removing our workaround (because your mitigation caused this to break every dialog, preventing it from even opening), we find that the dialogs now size to about 688px instead of 600px.

Can you evidence your testing process for this most recent change? What internal and community testing was carried out before pushing the change to production?

We're now back in a position where applications which have been stable for years are now non-functional, and we're once again working to implement mitigations and workarounds to changes pushed to production without notice.

We've reverted our change for the dialog and it should be as in this picture. image

Can you give me some details how the issue is ongoing?

christophepeerens commented 1 month ago

It's the same as before. The dialog appears as it was before the size limitation problem. The problem seems to have been solved.

AbshireMI commented 1 month ago

Can you give me some details how the issue is ongoing?

We have had to make further changes to set dialog width manually.

The main issue here is that you made changes to production without notice / discussion. Customers should have had opportunity to plan for change, as is best practice. There's also concern that you will re-introduce this change in the future, leading to a requirement to make further changes to our applications.

This needs to morph from being a discussion about a single technical issue to a wider governance discussion with the community, to ensure that these sorts of system-breaking issues cannot recur, or that if any potential breaking change is to be introduced that we have an opportunity to plan for, and resource, that change.

ymyqwe commented 1 month ago

Can you give me some details how the issue is ongoing?

We have had to make further changes to set dialog width manually.

The main issue here is that you made changes to production without notice / discussion. Customers should have had opportunity to plan for change, as is best practice. There's also concern that you will re-introduce this change in the future, leading to a requirement to make further changes to our applications.

This needs to morph from being a discussion about a single technical issue to a wider governance discussion with the community, to ensure that these sorts of system-breaking issues cannot recur, or that if any potential breaking change is to be introduced that we have an opportunity to plan for, and resource, that change.

Thanks for the suggestion. It's our fault. We'll have an internal discussion on how to make the release progress better

ymyqwe commented 1 month ago

We have a fix merged and the new v9 dialog will be rounded corner also support wide dialog. These change will be rolled out to customers in a couple of weeks.