search

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.55k stars 266 forks source link

Issues with purely static area Web Component: SupportTab (for SFSF Header) #1428

Closed codefactor closed 4 years ago

codefactor commented 4 years ago

Describe the bug

I am working on a new Web Component as part of the SuccessFactors WC Header. This web component is entirely within the static area, it is a floating button on the right side of the screen which the user can click that slides in the support IFRAME.

Here are 2 lines which reference the getStaticAreaItemDomRef function in this new Component:

  1. https://github.wdf.sap.corp/xweb/homepage4/blob/d33029e653961836bc46500bd2114e6946d9fa7e/src/webcomponents/SupportTab.js#L37
  2. https://github.wdf.sap.corp/xweb/homepage4/blob/d33029e653961836bc46500bd2114e6946d9fa7e/src/webcomponents/SupportTab.js#L70

There are 2 issues that I've found:

  1. In IE 11 and Edge the function getStaticAreaItemDomRef returns a Promise that is never resolved or rejected, this could be due to https://github.com/SAP/ui5-webcomponents/issues/1427 but I cannot be sure
  2. Errors happen if I remove the static get template function

For issue #2, I went ahead and added an empty template function so that I could work around the problem. This drives the _needsShadowDOM on UIElement to be true; however, for this component it is truly only in the static area and therefore does NOT need a shadow DOM. However, any time the component gets rendered for any reason, an error occurs.

    _updateShadowRoot() {
        let styleToPrepend;
        const renderResult = this.constructor.template(this); // <----

The error happens in the above pointed out line because this.constructor.template doesn't exist.

I might suggest that this function _updateShadowRoot should check _needsShadowDOM or check if the template function exists before calling it. Most likely this doesn't happen in any out of the box UI5 web components which probably all need shadow roots; however, for this particular component is a purely static area web component with no shadow root or template function at all.

As I mentioned, though, I am able to work out this issue by adding an empty (but useless) template. So it is really just issue #1 that is a blocker for me to continue, but I opened a separate ticket outside of https://github.com/SAP/ui5-webcomponents/issues/1427 to describe this other problem just in case the solution there does not resolve issue #1. Also reproducing this issue is much more complicated because you need a Web Component that is purely in the static area to reproduce it.

To reproduce Steps to reproduce the behavior:

  1. Go to '...'
  2. ...
  3. ...

Isolated example https://codesandbox.io/s/ui5-webcomponents-ypsxi

Expected behavior The support tab should work with or without the static template function. Also - in IE 11 and Edge the support tab should be clickable.

NOTE: I am not so sure what is wrong with the isolated example in IE 11 and Edge it is not working - I think something wrong with the poly fills. Please correct this on the sandbox if you can.

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

Context

Affected components (if known)

Log output / Any errors in the console

this.constructor.template is not a function
codefactor commented 4 years ago

Update -

I found that issue https://github.com/SAP/ui5-webcomponents/issues/1427 is indeed the root cause to issue #1 - by applying the simple fix that I mentioned in that issue it is resolved. So the only outstanding issue is the second one which happens in all browsers.

Issue #2 is fixed by changing the following code: https://github.com/SAP/ui5-webcomponents/blob/bc3de4d89dff12cce0e53782363d6de0563b39de/packages/base/src/UI5Element.js#L487-L495

    _updateShadowRoot() {
        if (!this.constructor._needsShadowDOM()) return;

This is one way to fix the issue.

fifoosid commented 4 years ago

Hi @codefactor

After merging the pull request, I wasn't able to reproduce the first issue that you have described. Here you can see the support tab working on Edge on my side(tested in on IE and it works as well):

example

Meanwhile I encountered another problem. The styles from SupportTab.css.js were not applied on Edge, so as a workaround I inlined them in SupportTab.lit.

Can you confirm that it works on your side as well?

Best regards, Filip

vladitasev commented 4 years ago

Closing due to a month of inactivity. In case the problem wasn't really solved, please let us know.