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

Configuration.waitForNoOverlayByJs vs waitByJsForNotOverlapped vs waitForNotOverlappedViaJs #85

Closed
yashaka opened this issue May 10, 2021 · 7 comments

Comments

@yashaka
Copy link
Owner

yashaka commented May 10, 2021

waitForNoOverlayByJs
+concise, kind of easy to explain because overlay is kind of self-explanatory
-inconsistent with inner ActualNotOverlappedWebElement that appears in error message if waiting failed
waitByJsForNotOverlapped
-not fully correct, the wait is not by js, but js is used to check if NotOverlapped...
+consistent with inner ActualNotOverlappedWebElement
waitForNotOverlappedByJs
-read like being overlapped by js, not some other element
waitForNotOverlappedViaJs
+consistent with inner ActualNotOverlappedWebElement
-kind of not ideal:) seems like... also can imply for somebody that wait is done by JS, but probably with lesser probability...

what to choose?

@yashaka
Copy link
Owner Author

yashaka commented May 10, 2021

yet, if waitForNotOverlappedViaJs does not give ideal explanation of what is happening...

why not used waitByJsForNotOverlapped?
yes, it's a bit lying about "how wait is implemented in context of waiting engine", but yet "checking for overlapped" is by js and is part of waiting process, and it's also consistent with other options like Configuration.TypeByJs, etc.

@yashaka
Copy link
Owner Author

yashaka commented May 10, 2021

Another thing... this waiting might relevant only for input type of actions (based on clear and sendKeys – Type, SetValue, PressEnter, etc.)

why then name it like waitByJsForNotOverlapped if more precise and less confusing name would be something like waitByJsForNotOverlappedOnInput ?

@davespoon
Copy link
Contributor

So from now we stay with WaitByJsForNotOverlapped?

@yashaka
Copy link
Owner Author

yashaka commented May 11, 2021

Still thinking :(

Latest today research revealed the following:

There three type of actions:

  1. Pointer-interaction
  • WebElement.Click
  1. Keyboard-interaction
  • Clear
  • SendKeys
  1. Actions
  • DoubleClick
  • MoveToElement (Hover)
  • ContextClick
  • etc.

All from above are reflected in some SeleneElement.* commands... All have built in waiting "till being passed (no errors appeared).

But.

Only 1 (click) have built into selenium treatment for:
1.1. +scroll into view (automatic)
1.2. +being overlapped (error will be thrown and so NSelene will continue waiting)

2 (clear, sendkeys) have built in treatment for
2.1 +scroll into view (automatic)
But has no treatment against
2.2. -being overlapped

3 (doubleclick, contextclick, hover) have nothing :)
3.1. -no automatic scroll into view
3.2. -no check for being overlapped

Now goes question... Do we need ? ...

  • A. one option for everything, like WaitByJsForNotOverlapped that will cover 2.2, 3.2 and even 3.1 (cause it can be considered as obvious - since we start waiting for not overlapped we should ensure that element is in the view)
  • B. one WaitByJsForNotOverlapped that will cover 2.2, 3.2 and separate for 3.1 - ScrollByJsIntoViewOnAllActions (can confuse others by meaning of "all" or "actions" terms... maybe then something like ScrollByJsAtPointInteractions)
  • C. WaitByJsForNotOverlapped that will cover 2.2, 3.2, and then specific option per each extra action: ScrollByJsIntoViewOnDoubleClick, ScrollByJsIntoViewOnHover, etc.
  • D. WaitByJsForNotOverlappedOnInput that will cover 2.2 for clear and sendkeys commands; WaitByJsForNotOverlappedAtPointInteractions for 3.2; ScrollByJsAtPointInteractions for 3.1

:)
Probably the easiest way would be for now go with solution A. And then in future rethink it, introduce something betwean B, C, D and deprecate WaitByJsForNotOverlapped, or just make it an additional option that in a fast way combines all others... This will be confusing a bit, but maybe handy in use at least for those who read the docs (additional explanation would be needed there...)

@yashaka
Copy link
Owner Author

yashaka commented May 11, 2021

Just one more idea of naming...
WaitByJsForNotOverlapped
vs
WaitForNoOverlapFoundByJs

hm... it happens to be pretty good!
almost same length, hence pretty concise, yet telling exactly what is done here by js - the overlap finding!

Seems like at least I like more the WaitForNoOverlapFoundByJs instead of WaitByJsForNotOverlapped

P.S. overlap seems to be better term that overlay, according to "google translate":)

@yashaka
Copy link
Owner Author

yashaka commented May 11, 2021

So... Let's really go this way (if no objections).

Rename current option to WaitForNoOverlapFoundByJs, make it work for both sendKeys/clear and Actions based commands like DoubleClick. Then in #86 add automatic scroll into view by js, when WaitForNoOverlapFoundByJs == true.

Then think about everything else:)

Yeah, maybe we need additional options mentioned in B, C, D above, but maybe the best implementation will be found later... Especially taking into account the ability to implement hooks in NSelene. Because all we do here - is "hooking selenium webdriver behaviour by adding extra features" and we do this in a "hardcode" style - making it buit-in yet customizable. But if we implement a way to hook anything in NSelene - then anybody can do this on their own without dependency to NSelene options like WaitForNoOverlapFoundByJs. So in long-time-perspective, hooks is a more powerfull feature. Yet also more complicated in implementation. That's why let's implement WaitForNoOverlapFoundByJs for now to gain the most benefit for projects where it will help to treat overlays, and then we think on perfect implementation through hooks and other options...

davespoon pushed a commit to davespoon/NSelene that referenced this issue May 11, 2021
@davespoon
Copy link
Contributor

Just one more idea of naming...
WaitByJsForNotOverlapped
vs
WaitForNoOverlapFoundByJs

hm... it happens to be pretty good!
almost same length, hence pretty concise, yet telling exactly what is done here by js - the overlap finding!

Seems like at least I like more the WaitForNoOverlapFoundByJs instead of WaitByJsForNotOverlapped

P.S. overlap seems to be better term that overlay, according to "google translate":)

Feels more smooth in context of human language =)

yashaka added a commit that referenced this issue May 12, 2021
WaitByJsForNotOverlapped renamed to WaitForNoOverlapFoundByJs; covered hover and double click with tests - happened that they does not support treatments against overlay by default, they just passes under overlay with no effect – this adds one more argument for making WaitForNoOverlapFoundByJs = true by default... yet probably we can't do this right now, without proper docs or extra plugins for mobile apps with NSelene via Appium...
github-actions bot added a commit that referenced this issue May 12, 2021
WaitByJsForNotOverlapped renamed to WaitForNoOverlapFoundByJs; covered hover and double click with tests - happened that they does not support treatments against overlay by default, they just passes under overlay with no effect – this adds one more argument for making WaitForNoOverlapFoundByJs = true by default... yet probably we can't do this right now, without proper docs or extra plugins for mobile apps with NSelene via Appium...
@yashaka yashaka closed this as completed May 12, 2021
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

No branches or pull requests

2 participants