Mayvenn / limo

A wrapper around selenium webdriver
Eclipse Public License 1.0
25 stars 4 forks source link

scroll-to behavior in Firefox #8

Closed mainej closed 4 years ago

mainej commented 6 years ago

Hi @jeffh and Mayvenn crew!

I'm evaluating limo for a project and have run into a problem with scroll-to in Firefox. We have a position: fixed header. In Firefox, limo's scroll-to uses scrollIntoView(). The default for scrollIntoView is to scroll so "the top of the element will be aligned to the top of the visible area of the scrollable ancestor". With the fixed header, this puts the scrolled element under the header, where it isn't clickable.

scrollIntoView accepts options to control how it scrolls. In my case, scrollIntoView(false), or equivalently scrollIntoView({block: "end", inline: "nearest"}) keeps the scrolled element on screen, because it tries to align the bottom of the element with the bottom of its ancestor. block: "center" also works, though block: "nearest" doesn't.

So, what do you think the best way to handle this would be? I can imagine scenarios where you'd need any one of the scroll behaviors. So maybe scroll-to should be configurable. One tricky part is that click and a few other helpers use scroll-to, so you'd have to be able to pass the configuration through those helpers. And of course in Safari, Chrome and other browsers, limo uses the non-standard scrollIntoViewIfNeeded, which doesn't support as many options as scrollIntoView.

Some possibilities:

I'm not really pleased with any of these. Do you have other ideas? If we settle on something, I'd be happy to submit a pull request.

Cheers!

jeffh commented 6 years ago

Hey @mainej,

Although the extend-protocol seems like an interesting approach. I don't know if that would eliminate global state (or keep it in the driver instance), but that would probably be best. Otherwise having another dynamic var would suffice (sadly).

I'm still thinking about what a new redesigned version of the API (v2) would look like. Lots of the current API problems come from an (over) reaction to all the wrapped selenium types and its taxi namespace in the old versions of clj-webdriver that we were using. I do think the protocol + extend style is the future underlying approach, but I would like to experiment a few things before committing to that design.

Thanks, Jeff

jeffh commented 4 years ago

This issue is probably a bit stale now. But scroll-to now supports passing in block, inline and behavior parameters. It's currently set to "center". Chrome and Firefox have updated to support the Draft W3C specification of scrollIntoView. It doesn't solve that click implicitly calls this which is a fundamental design flaw of the current limo API.

"center" seems like the best default since it minimizes the likelihood of fixed headers/footers from conflicting (based on our test suite). Just looking at Chrome's implementation, it seems to attempt to center the element on the page whenever possible.

Going forward, I'm working on a v2 version of the API that is going to be more protocol-oriented to try and help provide more flexibility in opting to have automatically scrolling behavior or not.

It's still very early stages and will likely dramatically change in the coming commits until most of the existing features have been ported to v2. My overall goal in v2 is to separate on the "do this thing around every operation so that its easier to use correctly" to be configurable at the initialization level.

Thanks, Jeff