Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(touchevent) - improve shadow swiping class check #4732

Merged
merged 3 commits into from
Jul 26, 2021

Conversation

weiz18
Copy link
Contributor

@weiz18 weiz18 commented Jul 2, 2021

Previously, we modified the targetEl to ensure element.closest(swipingClass) to always finding the correct element. But it does not work cross browser since event path is different from safari to chrome. This refactor introduce a new method to find the closest matching element with nested shadowRoot, so its now working the same way as the non shadow components.

So swipe action trigger inside shadow-component will be blocked if either <Element> or <Inner-element> contains no swiping class.
<Element class="no-swiping">
<Inner-element class="no-swiping">
<shadow-comp />
<Inner-element>
</Element>

@weiz18 weiz18 closed this Jul 2, 2021
@weiz18 weiz18 reopened this Jul 5, 2021
@nolimits4web
Copy link
Owner

Hey @weiz18, thanks! But can you give me some minimal live example of what it fixes (where now it will be broken)? Just for better understanding what we are fixing here :)

@weiz18
Copy link
Contributor Author

weiz18 commented Jul 14, 2021

Sure. will attach a minimal live example when i have some time. Basically, current implementation of finding the closest target on shadow root only works with one level. So if you have a deep nested dom structure where the no swiping class is on the outer most element, it will fail to find it in the path.

<div class="swiper-no-swiping"><shadow-comp><shadow-comp-2></shadow-comp-2></shadow-comp></div>, 

@nolimits4web
Copy link
Owner

Still not sure what is the issue, because right now tested the following:

    <div class="swiper-slide">
      <span class="swiper-no-swiping" style="display: inline-block;">
        <custom-1>
          <custom-2>
            <p style="line-height: 1;">Hello world</p>
          </custom-2>
        </custom-1>
      </span>
    </div>

And everything works as expected for me in Safari, Chrome and FF. There is no swiping over this elements

@weiz18
Copy link
Contributor Author

weiz18 commented Jul 26, 2021

Hey, @nolimits4web sorry for the late response, was away for a couple of days. I have attached a demo reproducing the issue. https://jsfiddle.net/qhzwom26/31/ . as you can see in the demo, the child element is still moving despite no-swiping class being set on the parent element. I think the key here is to set the shadow root mode to open in order to reproduce. let me know if you need more info.
thanks

@nolimits4web nolimits4web merged commit 422e321 into nolimits4web:master Jul 26, 2021
@nolimits4web
Copy link
Owner

Merged, thanks!

@weiz18
Copy link
Contributor Author

weiz18 commented Jul 26, 2021

nice. thanks a lot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants