Skip to content

Selector widgets#96

Merged
tdhock merged 93 commits intomasterfrom
selector-widgets
Sep 8, 2015
Merged

Selector widgets#96
tdhock merged 93 commits intomasterfrom
selector-widgets

Conversation

@kferris10
Copy link
Copy Markdown
Collaborator

Here's a start to creating a widget for each selector variable. It's a work in progress as it only works for this one example, but I wanted to show you guys where I'm currently at.

p1 <- ggplot() +
  geom_point(aes(Sepal.Length, Sepal.Width, clickSelects = Species),
             data = iris) +
  ggtitle("Sepal Data")
p2 <- ggplot() +
  geom_point(aes(Petal.Length, Petal.Width, colour = Species, 
                 showSelected = Species), data = iris) +
  ggtitle("Petal Data")

viz <- list(sepal = p1, petal = p2)
animint2dir(viz)

It also works with multiple selection

viz <- list(sepal = p1, petal = p2, 
            selector.types = list(Species = "multiple"))
animint2dir(viz)

Live example

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When clicking the Show Species selector button, I want to update the selector_table so that it is no longer hidden. But this approach isn't working. I think that there's a problem with the d3.selectAll(".selector_table").select("#" + this.id) statement, but I'm not sure what the problem is. If anybody has any ideas, please let me know.

@tdhock
Copy link
Copy Markdown
Owner

tdhock commented Jul 13, 2015

would it be possible to use http://brianreavis.github.io/selectize.js/ ?

the autocomplete menu would be especially useful for selecting from a long list of things (for example countries in the worldbank viz).

@kferris10
Copy link
Copy Markdown
Collaborator Author

I'm sure it is, but I've never used that before so it might take me a bit. To start, I'd have to add selectize.js and selectize.css to /Animint/inst/htmljs/vendor/ right? Or is there a more appropriate location?

@tdhock
Copy link
Copy Markdown
Owner

tdhock commented Jul 15, 2015

yes that is an appropriate place to store the selectize css and js files. github's repos code analysis ignores files under the /vendor subdirectory (so then the percent of JS and CSS code in animint is not artificially elevated).

@cpsievert
Copy link
Copy Markdown
Collaborator

Looking great @kferris10!

We'll have to address and fix (1), and if I were to guess, it may be that a variable is being defined without var (and thus is being introduced to the global environmnent).

@kferris10
Copy link
Copy Markdown
Collaborator Author

This latest push takes care of the issue with everything de-selected. Now we're just down to the global variables issue.

Here's the output I get from diff.vars in this test.

> tests_run(filter = "global")
global variables : Called from: eval(expr, envir, enclos)
Browse[1]> diff.vars
[1] "animint"                      "jquery_thingy_array"         
[3] "jQuery1113007219210239316454"

I have no idea why jQuery1113007219210239316454 is in there. My only guess is that it comes from jQuery itself? If don't script jquery in in index.html, then it disappears from diff.vars.

The jquery_thingy_array is currently in there because I need to access it in both the d3.json() part of animint and in update_selector() and I can't figure out how to refer to it in both places without making it a global variable. Does that make sense?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cpsievert I could use your help here too. Do you know how to interact with selectize through the test?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you could do this with a combination of RSelenium's findElement and clickElement methods, but I've found that using executeScript is often faster. This page documents those and other handy methods -> http://www.rdocumentation.org/packages/RSelenium/functions/remoteDriver-class

I was able to execute this in my Firefox console for the example you link to above:

// introduce the dropdown content of the 'vs' selector to the DOM
document.getElementsByClassName("selectize-input")[0].click();
// store the content
var con = document.getElementsByClassName("selectize-dropdown-content")[0]
// click on the value '1' (which happens to be the second node)
con.childNodes[1].click()

So I'd try executing this JS code via remDr$executeScript("...")

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't figure how to click con.childNodes without it crashing. This does run fine in my browser though.

1. Error: Updating cyl widget shows/hides points -----------------------------------
     Summary: UnknownAn unknown server-side error occurred while processing the command.
     class: org.openqa.selenium.WebDriverException
1: withCallingHandlers(eval(code, new_test_environment), error = capture_calls)
2: eval(code, new_test_environment)
3: eval(expr, envir, enclos)
4: remDr$executeScript("\n    // introduce the dropdown content of the 'vs' selector to the DOM\n    document.getElementsByClassName('selectize-input')[0].click();\n    // store the content\n    var con = document.getElementsByClassName('selectize-dropdown-content')[0];\n    // click on the value '1' (which happens to be the second node)\n    con.childNodes[1].doubleclick();\n                      ") at test-selector-widgets.R:50
5: queryRD(paste0(serverURL, "/session/", sessionInfo$id, "/execute"), "POST", qdata = toJSON(list(script = script, 
       args = args)), json = TRUE)
6: checkStatus()
7: stop(errMessage, call. = FALSE)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you please write an expectation for this test?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still need a little bit of help writing this test. Maybe @cpsievert has some ideas why con.childNodes[1].click(); returns an error when using tests_run()?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of using executeScript, you may consider using clickID, remDr$findElement, e$clickElement, and e$sendKeysToElement as in test-widerect.R

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kferris10

Looks like HTMLElementObject.click() is (sort of) on the TODO list for phantomjs 2.0, but I couldn't get it to work locally (even worse, some other test crashes it)

I suppose we should go with @tdhock suggestion, but note that
those tests are skipped on phantomjs, so you'll probably want to skip them here as well (and verify with firefox locally)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. I'm pretty sure that I could replicate your example @tdhock in test-selector-widgets if that would be better.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe your test in test-selector-widgets.R
#96 (comment) is no
longer necessary? I added the tests yesterday in test-widerect.R and that
seems to exercise most of the selectize features I would use.

On Mon, Aug 31, 2015 at 8:11 PM, Kevin Ferris notifications@github.com
wrote:

In tests/testthat/test-selector-widgets.R
#96 (comment):

  • expect_equal(length(vs_widget), 1)
  • testing that the cyl widget is present

  • cyl_widget <- getNodeSet(info$html, '//select[@id="cyl_input"]')
  • expect_equal(length(cyl_widget), 1)
    +})

+test_that("Updating cyl widget shows/hides points", {

  • remDr$executeScript("
  • // introduce the dropdown content of the 'vs' selector to the DOM
  • document.getElementsByClassName('selectize-input')[0].click();
  • // store the content
  • var con = document.getElementsByClassName('selectize-dropdown-content')[0];
  • // click on the value '1' (which happens to be the second node)
  • con.childNodes[1].click();
  •                  ")
    
    +})

Okay. I'm pretty sure that I could replicate your example @tdhock
https://github.com/tdhock in test-selector-widgets if that would be
better.


Reply to this email directly or view it on GitHub
https://github.com/tdhock/animint/pull/96/files#r38374678.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds fine to me. I can delete test-selector-widgets.R and merge it tonight.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok but we should take a look at fixing the other two test failures before merging.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad. I wasn't paying close enough attention. I'll look at those as soon as I can

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will have a look later today too.

@tdhock
Copy link
Copy Markdown
Owner

tdhock commented Aug 31, 2015

Hey @kferris10 this selector widget functionality looks like it is working exactly as I imagined it, on the WorldBank data viz that I looked at. Great job! Anything else to do before merging with master?

@caijun
Copy link
Copy Markdown
Collaborator

caijun commented Sep 3, 2015

The shiny test should be disabled due to the problem of PhantomJS. ariya/phantomjs#11526

@cpsievert
Copy link
Copy Markdown
Collaborator

That's a bummer, go ahead and re-disable.

@kferris10
Copy link
Copy Markdown
Collaborator Author

This takes care of the malaria tests for me. It was the same problem we had with some of the malaria tests where I forgot to remove periods from variable names 2349c9b

Sorry this took a few days. If @cpsievert could tell the Twins to stop winning, that might help me find some more free time :)

@tdhock
Copy link
Copy Markdown
Owner

tdhock commented Sep 8, 2015

Thanks for the fixes @kferris10

Tests pass on my machine using firefox.

I updated the version and will merge with master.

tdhock pushed a commit that referenced this pull request Sep 8, 2015
@tdhock tdhock merged commit 29efdfc into master Sep 8, 2015
@tdhock tdhock deleted the selector-widgets branch September 8, 2015 16:10
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.

4 participants