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

feat(scripting): add Element class with `offset` method #422

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@mike1808
Contributor

mike1808 commented Mar 20, 2016

I've tried to implement Element class suggested in #380
HTML element can be retrieved using #select method on the splash object.

local element = splash:select('.container .title')

In this PR I've implemented only 1 method for this class: #offset. It returns a dictionary with top and left offset value like in jQuery#offset method.

There is no tests and docs update. Only one script example. I'll write them if my assumptions what'd be done is right.

Is this what #380 have suggested or I'm going to the wrong direction?

P.S. This PR was creating for my proposal for GSoC 2016.

@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Mar 20, 2016

Current coverage is 85.59%

Merging #422 into master will increase coverage by +0.05% as of 94bf237

@@            master    #422   diff @@
======================================
  Files           39      39       
  Stmts         4628    4652    +24
  Branches       632     632       
  Methods          0       0       
======================================
+ Hit           3959    3982    +23
- Partial        157     159     +2
+ Missed         512     511     -1

Review entire Coverage Diff as of 94bf237

Powered by Codecov. Updated on successful CI builds.

codecov-io commented Mar 20, 2016

Current coverage is 85.59%

Merging #422 into master will increase coverage by +0.05% as of 94bf237

@@            master    #422   diff @@
======================================
  Files           39      39       
  Stmts         4628    4652    +24
  Branches       632     632       
  Methods          0       0       
======================================
+ Hit           3959    3982    +23
- Partial        157     159     +2
+ Missed         512     511     -1

Review entire Coverage Diff as of 94bf237

Powered by Codecov. Updated on successful CI builds.

@kmike

This comment has been minimized.

Show comment
Hide comment
@kmike

kmike Mar 21, 2016

Member

Hey @mike1808,

The PR looks fine for what it does, but I'm reclucant to merge it without a clear vision on API we're going to provide. You're asking right questions in #380 (comment), but there are more of them, e.g.:

  • what to do in cases where a selector returns multiple elements?
  • what if an user has a DOM element selected in a JS function - how to convert it to Element?
  • is it possible to pass Element back to JavaScript code?
Member

kmike commented Mar 21, 2016

Hey @mike1808,

The PR looks fine for what it does, but I'm reclucant to merge it without a clear vision on API we're going to provide. You're asking right questions in #380 (comment), but there are more of them, e.g.:

  • what to do in cases where a selector returns multiple elements?
  • what if an user has a DOM element selected in a JS function - how to convert it to Element?
  • is it possible to pass Element back to JavaScript code?
feat(scripting): add Element class with `offset` method
Add Element class to use in scripts which allow calling HTML element-specific
methods just in Lua. HTML elements can be selected using #select method
on `splash` object.

Currently, only one method is added: `offset`.

@mike1808 mike1808 closed this Aug 17, 2016

@mike1808 mike1808 deleted the mike1808:element-class branch Aug 17, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment