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

saw htmlwidget on the todo #1

Open
timelyportfolio opened this issue Aug 21, 2015 · 25 comments
Open

saw htmlwidget on the todo #1

timelyportfolio opened this issue Aug 21, 2015 · 25 comments

Comments

@timelyportfolio
Copy link
Contributor

I saw htmlwidget on the todo and I'd love to help. I will spend some time today getting acquainted with this fine piece of work.

@jbkunst
Copy link

jbkunst commented Aug 21, 2015

👍

@cmpolis
Copy link
Owner

cmpolis commented Aug 23, 2015

Terrific - thanks for your interest and help! The demo code is not available - this project is a complete rewrite, which will hopefully be usable soon. I'm excited about having an R interface and being able to easily plug in data frames.

I haven't come up with an api or thought about the implementation, but I'd imagine it being similar to the DataTable htmlwidget. I'm a bit of an R novice, so any help is greatly appreciated! 🙌

@abresler
Copy link

This would be fantastic - happy to try to help as well. Try to taking a look at the very nice port of PivottableJS which made its way into a great htmlwidget you can see here

@timelyportfolio
Copy link
Contributor Author

I see that you are making solid progress. When I try to run on my side, I get this:

image

The constructor on my side would be new Datacomb({...}) but then I get other errors. Am I missing something obvious before I dive deep into the code?

image

@cmpolis
Copy link
Owner

cmpolis commented Sep 20, 2015

Thanks, things are coming along nicely!

It looks like the first screenshot was coming from things not being built, does $ npm run build complete without error (this step was previously not in the Readme, sorry!)? I will document this better, but for now the best sample of how to init an interface is in /demo/datacomb/demo.js

@cmpolis
Copy link
Owner

cmpolis commented Sep 20, 2015

Also, I have some htmlwidget/R code from a previous version of datacomb that I will try to carry over to this iteration, this week!

@timelyportfolio
Copy link
Contributor Author

Something on my end, so now the demo is working, and it is beautiful. I had run npm run build but I think my setup was confused. I'd love to help implement the htmlwidget. I guess following along on Github will be best.

@abresler
Copy link

Oh this would be so so amazing!!


Alex Bresler
[email protected]
​www.asbcllc.com​
917-455-0239​ (cell)​
On Sep 20, 2015 9:47 AM, "timelyportfolio" [email protected] wrote:

Something on my end, so now the demo is working, and it is beautiful. I'd
love to help implement the htmlwidget. I guess following along on Github
will be best.


Reply to this email directly or view it on GitHub
#1 (comment).

@cmpolis
Copy link
Owner

cmpolis commented Sep 22, 2015

I put in a quick, first pass for this! I have a working knowledge of R, but am somewhat clueless on defining the proper API, docs, etc... so feel free to put in updates! Currently, the API is fairly basic; it just consumes a dataframe and column and label params. Also, let me know if you are able to install and get this up and running through R.

devtools::install_github('cmpolis/datacomb')
library(datacomb)
Datacomb(iris, columns=c('Sepal.Length','Sepal.Width', 'Petal.Length', 'Petal.Width', 'Species'))

@timelyportfolio
Copy link
Contributor Author

I'll start playing with it now. This looks like a great start. I would think eventually a different repo will help avoid confusion, or at a minimum, I think we can use a directory pkg that R will know to use for install.

@timelyportfolio
Copy link
Contributor Author

@cmpolis I toyed with it a bit at https://github.com/timelyportfolio/datacomb/tree/R.

# devtools::install_github('timelyportfolio/datacomb@R', subdir = 'pkg')

library(datacomb)
?Datacomb
Datacomb(mtcars)

Datacomb(iris, rowLabel = "Species")

data(diamonds, package = "ggplot2")
Datacomb(diamonds)

This is going to be awesome!

@abresler
Copy link

Guys had to just chime in and say this is every bit as amazing I hoped; holy CRAP

@cmpolis
Copy link
Owner

cmpolis commented Sep 23, 2015

@timelyportfolio 👍 terrific - thanks for the updates!!! Can I merge in/do you want to put in a pr?

@timelyportfolio
Copy link
Contributor Author

@cmpolis, easy when you've done such an amazing job. I hesitated to submit pr before making sure that a blended repo wtih JS/node + R + maybe python + who knows what other implementations would be a good strategy, or if each implementation should have its own repo to avoid confusion. Would a datacomb organization be an option?

@cmpolis
Copy link
Owner

cmpolis commented Sep 23, 2015

I would shy away against creating an organization, just because it will add some complexity in terms of managing repos, users, what is the process for bringing a repo into the org, etc... the main non-web interface that I am interested in is R; I think that's fine to leave in here. The list other potential interfaces/package management systems is somewhat endless, so I'd like to not get too involved and focus on the actual interface 😎 (and R functionality b/c I will be using this myself). If others want to make repos that plug into datacomb or put in a pr for [bower, component,...].json, that's great!

@cmpolis
Copy link
Owner

cmpolis commented Sep 23, 2015

that being said, one of the great things about your pr is having everything up a level, in /pkg! I will likely reorganize this project to have a more san dir structure:

/src/[js,css,hbs,etc...]/
/dist
/lib
/test
...

@timelyportfolio
Copy link
Contributor Author

Definitely helps to understand this, and I appreciate the insight. /pkg requires the subdir argument, but I think the advantages will be worth the extra argument, especially if you are thinking of a san dir structure. Do you want me to wait to see the new directory structure before a pull?

@cmpolis
Copy link
Owner

cmpolis commented Sep 23, 2015

+1 don't mind subdir arg; I think most people will be copy/pasting or looking at some sample code anyways to install. Feel free to put in your pr - I'll cleanup and do my refactoring later.

Few things re the DESCRIPTION file:

  • can we remove the R dependency or lower the version number? I had a friend try to install this today and was seeing issues because of a fairly recent version being required.
  • what else should we do with the depends, imports... properties?
  • please add yourself on there!

@timelyportfolio
Copy link
Contributor Author

This doesn't seem to be working anymore http://timelyportfolio.github.io/buildingwidgets/week38/example01.html. Scrolling stops at 1000 records. Do you mind glancing at it and making sure I'm not missing something obvious?

@cmpolis
Copy link
Owner

cmpolis commented Sep 25, 2015

Yes, there seems to be an issue w/ row reuse. If you bump this parameter, I think it will be fixed.

@cmpolis
Copy link
Owner

cmpolis commented Sep 25, 2015

@timelyportfolio I think this might fix (or fix part of) your issue: e730b14 ... still working out some kinks in this thing

@timelyportfolio
Copy link
Contributor Author

Yes, thanks that fixes the div recycle, but I now seem to have a new problem. I'll try to describe it in words and illustrate with a GIF. Focus on hover works great until we scroll past the initial available div pool. Then focus on hover moves to the bottom of the screen, and the screen becomes blank.

datacomb_focus_problem

I'm going to try to debug, but afraid my skills won't be sufficient for the task.

@timelyportfolio
Copy link
Contributor Author

Maybe this is a clue as style goes from > 2000000px to 0px. I'll keep digging.

datacomb_focus_problem_elements

@timelyportfolio
Copy link
Contributor Author

I think I have traced the problem to this line 102 in smart-table-scroll

if(this.lastMidNdx === midNdx) { this.isUpdating = false; return; }

because this line 178

this.data[ndx].__node.style.top = this.data[ndx].__top + 'px';

sets top to 0px++ and then I think expects line 182 to properly set top.

However, it could be that line 178 is just not setting top correctly.

this.data[ndx].__node.style.top = this.data[ndx].__top + 'px';

Changing lastMidNdx seems to fix the scroll to blank problem, but unfortunately also seems to cause a loss in performance.

  for(ndx = 0; ndx < oldNodes.length; ndx++) {
    this.data[ndx].__node = oldNodes[ndx];
    this.updateRow(this.data[ndx], this.data[ndx].__node);
    this.data[ndx].__node.style.top = this.data[ndx].__top + 'px';
  }
  this.bottomEl.style.top = this.totalHeight + 'px';
  this.lastMidNdx = 0;
  this.isUpdating = false;
  this.updateVisibleRows();

@cmpolis
Copy link
Owner

cmpolis commented Sep 27, 2015

Thanks for the detailed report; looking into it!

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

4 participants