NSelene icon indicating copy to clipboard operation
NSelene copied to clipboard

JsClick implementation details (aka ClickViaJs or ClickByJs)

Open yashaka opened this issue 4 years ago • 6 comments

In #49 we started to add ClickByJs implementation...

This was the more or less original proposal by @wjgerritsen-0001 :

            public static SeleneElement JsClickWithOffset(this SeleneElement selement, int xOffset, int yOffset)
            {
                selement.ExecuteScript(@"
                    const xOffset = args[0];
                    const yOffset = args[1];
                    element.dispatchEvent(new MouseEvent(
                            'click',
                            {
                                bubbles: true,
                                cancelable: true,
                                clientX: element.getClientRects()[0].left + xOffset,
                                clientY: element.getClientRects()[0].top + yOffset,
                                view: window,
                            }
                    ));", 
                    xOffset, 
                    yOffset);
                return selement;
            }
        }

Here are some open points to clarify:

  1. Should we count offset from the center of an element like in selenide from java: https://github.com/selenide/selenide/blob/master/src/main/java/com/codeborne/selenide/commands/Click.java#L47

pros (for counting from the center):

  • consistent with selenide from java, smoother migration from one ecosystem to another if needed
  • more obvious (people are used to the click behavior from webdriver that clicks on the center of element)
  • consistent with "js click without offset" - this method we also need... and it should nevertheless click on the center for similarity with webdriver click

cons:

  • clicking outside of an element becomes less concise, compare:
    • from center: var foo = S("#foo"); var size = foo.Size; foo.ClickWithOffset(size.Width, size.Height)
      • yet this is more verbose way is more universal we can click outside whatever side of an element...
    • from top left corner: S("#foo").ClickWithOffset(-10, -10)
      • yet this works only if we want to click to the left or above the element...
  1. Should we name the method as JsClick(this SeleneElement element, int xOffset=0, int yOffset=0), also in this way being more consistent with Selenide from Java

pros

  • more concise
  • consistent with selenide from java
  • one method instead of two ones (we will have nevertheless to add JsClick with no params then for js click in the center of an element

cons

  • maybe less obvious to understand, compare:
    • JsClick(100, 100)
    • JsClickWithOffset(100, 100)

yashaka avatar Jun 24 '20 18:06 yashaka

For now I tend to choose the JsClick(this SeleneElement element, int xOffset=0, int yOffset=0) version, counting offset from the center

yashaka avatar Jun 24 '20 18:06 yashaka

What about adding both:

  • JsClick(this SeleneElement element, int xOffset=0, int yOffset=0) version, counting offset from the center
  • JsClickWithTopLeftOffset(this SeleneElement element, int xOffset=0, int yOffset=0) or even also adding:
  • JsClickWithBottomRightOffset(this SeleneElement element, int xOffset=0, int yOffset=0)

then another open point: JsClickWithTopLeftOffset vs JsClickWithOffsetFromTopLeft

:)

yashaka avatar Jun 24 '20 18:06 yashaka

I like the methodnames in your last comment, top one should becom JsClickCenter! Very explicit, no guessing where the click will come.

wjgerritsen-0001 avatar Jun 26 '20 11:06 wjgerritsen-0001

            public static SeleneElement JSClickCenter(this SeleneElement element, int xOffset=0, int yOffset=0)
            {
                element.ExecuteScript(@"
                    var xOffset = args[0];
                    var yOffset = args[1];

                    var rect = element.getBoundingClientRect();
                    element.dispatchEvent(new MouseEvent('click', {
                        'view': window,
                        'bubbles': true,
                        'cancelable': true,
                        'clientX': rect.left + rect.width/2 + xOffset,
                        'clientY': rect.top + rect.height/2 + yOffset 
                    }));
                    ", xOffset, yOffset);
                return element;
            }

wjgerritsen-0001 avatar Jun 26 '20 11:06 wjgerritsen-0001

Yeah, it's more verbose, very explicit:) But then "JsClickCenter" will not be fully consistent with a common webdriver "Click" Let's keep the "base" JsClick named same way as selenium webdriver Click - without additional ending. It should be known to everybody that selenium webdriver clicks on center by default...

yet, for self-documentating purposes, let's code into method signature some hints like this:

public static SeleneElement JsClick(this SeleneElement element, int centerXOffset=0, int centerYOffset=0)

and then additional methods we can add with more explicit names to distinguish from the base one:

public static SeleneElement JsClickWithTopLeftOffset(this SeleneElement element, int xOffset=0, int yOffset=0);
public static SeleneElement JsClickWithBottomRightOffset(this SeleneElement element, int xOffset=0, int yOffset=0);

yet for these additionally methods we should better think on names...

if named like JsClickWithTopLeftOffset, then probably we should not use default arguments

public static SeleneElement JsClickWithTopLeftOffset(this SeleneElement element, int xOffset, int yOffset);

otherwise, name like in your proposal with JsClickCenter:

public static SeleneElement JsClickTopLeft(this SeleneElement element, int xOffset=0, int yOffset=0);
public static SeleneElement JsClickBottomRight(this SeleneElement element, int xOffset=0, int yOffset=0);

or maybe:

public static SeleneElement JsClickTopLeft(this SeleneElement element, int xOffset=1, int yOffset=1);
public static SeleneElement JsClickBottomRight(this SeleneElement element, int xOffset=-1, int yOffset=-1);

...

yashaka avatar Jun 29 '20 12:06 yashaka

Help wanted: needed a test for JsClick to verify that offset counts from the center...

something like this:

        // TODO: make it work and pass:)
        // [Test]
        // public void JsClick_ClicksWithOffsetFromCenter()
        // {
        //     Given.OpenedPageWithBody(@"
        //         <a id='to-first' href='#first'>go to Heading 1</a>
        //         </br>
        //         <a id='to-second' href='#second'>go to Heading 2</a>
        //         <h2 id='second'>Heading 1</h2>
        //         <h2 id='second'>Heading 2</h2>"
        //     );

        //     var toSecond = S("#to-second");
            
        //     toSecond.JsClick(0, -toSecond.Size.Height);
            
        //     Assert.IsTrue(Selene.GetWebDriver().Url.Contains("first"));
        // }

but make it work, and check also the "x" axis...

yashaka avatar Jul 06 '20 22:07 yashaka