SAP / ui5-webcomponents

UI5 Web Components - the enterprise-flavored sugar on top of native APIs! Build SAP Fiori user interfaces with the technology of your choice.
https://sap.github.io/ui5-webcomponents/
Apache License 2.0
1.56k stars 267 forks source link

Edge: ui5-shellbar - shellbar items are broken #1427

Closed codefactor closed 4 years ago

codefactor commented 4 years ago

Describe the bug In Edge browser, the shellbar items are broken due to the in the shellbar. The error happens on this line: http://github.com/SAP/ui5-webcomponents/blob/bc3de4d89dff12cce0e53782363d6de0563b39de/packages/fiori/src/ShellBar.js#L816 JSON.stringify() when containing the avatar causes a self-reference error.

To reproduce Steps to reproduce the behavior:

  1. Use Edge
  2. Go To https://sap.github.io/ui5-webcomponents/playground/components/ShellBar/

Isolated example Not Needed

Expected behavior The shellbar should be visible

Screenshots If applicable, add screenshots to help explain your problem.

Context

Affected components (if known)

Log output / Any errors in the console

0: Circular reference in value argument not supported
MapTo0 commented 4 years ago

Which version of Edge are you using? Is the issue available with Chromium Edge?

codefactor commented 4 years ago

@MapTo0 ,

Microsoft Edge 44.17763.831.0

This is the default Edge browser that all SuccessFactors employees get. In addition I think a similar issue happens in IE 11.

I also found for some reason that the promise returned by this.getStaticAreaItemDomRef() is never returning in Edge browser.

codefactor commented 4 years ago

Here is the same error in IE 11:

SCRIPT5034: Circular reference in value argument not supported

When I was debugging the issue - the following line was problematic:

const isDifferent = JSON.stringify(this._itemsInfo) !== JSON.stringify(newItems);

The JSON.stringify() is given an array of newItems - and the item that corresponded with the Avatar item had some sort of circular reference. You can manually explore this one object and see that it is has a large tree from there. The other items are fine. I imagine the recent changes to start using the ui5-avatar has introduced this problem.

I guess there might be better ways to see if any new items were introduced rather than completely stringifying the item array and doing a string comparison.

codefactor commented 4 years ago

@MapTo0 ,

Looks like you can change the following line: https://github.com/SAP/ui5-webcomponents/blob/bc3de4d89dff12cce0e53782363d6de0563b39de/packages/fiori/src/ShellBar.js#L795

   show: !!this.profile,

I think the show property is meant to be a boolean, so if you change it to show: !!this.profile, it should fix the issue.

codefactor commented 4 years ago

Update - while the before mentioned fix does seem to fix the issue partially - there remains a few other issues that only happen in IE 11 and Edge:

  1. Some of the shellbar icons are not visible as before
  2. The avatar photo is no longer clickable, the click event on profile is not fired and the photo does not have any cursor effect when you hover on it
  3. The shellbar icons are always collapsed into the overflow toolbar even though it is at desktop width.
MarcusNotheis commented 4 years ago

We are facing a similiar problem when using the avatar component in a ShellBar as profile slot:

ShellBar.js:816 Uncaught (in promise) TypeError: Converting circular structure to JSON
    --> starting at object with constructor 'HTMLElement'
    |     property '__reactInternalInstance$puiksydu36k' -> object with constructor 'FiberNode'
    --- property 'stateNode' closes the circle
    at JSON.stringify (<anonymous>)
    at HTMLElement._updateItemsInfo (ShellBar.js:816)
    at HTMLElement._overflowActions (ShellBar.js:624)
    at HTMLElement.onAfterRendering (ShellBar.js:485)
    at HTMLElement._render (UI5Element.js:480)
    at Function.renderWebComponents (RenderScheduler.js:82)
    at Function.runRenderTask (RenderScheduler.js:62)
    at Function.renderImmediately (RenderScheduler.js:44)
    at HTMLElement.connectedCallback (UI5Element.js:112)

Looks like this might be related to react as it is not happening in your playground?

Environment Info: Browser: Chrome 80.0.3987.163 OS: MacOS Catalina 10.15.4

Isolated Example: https://codesandbox.io/s/dawn-fog-q36zv?file=/src/App.js (You have to download it and run it locally)

vladitasev commented 4 years ago

Thanks for the reports! We'll look into this asap.

Regards

vladitasev commented 4 years ago

https://github.com/SAP/ui5-webcomponents/pull/1438