fix for Uncaught ReferenceError: p is not definer when expanding groups

[bchr02]

In reference to issue #86

Now that I have had a chance to test this out, I see that the issue has been resolved but now there is a new bug which doesn't allow an expanded group to close and reopen without throwing an error in the console: Uncaught ReferenceError: p is not defined. This push request fixed that error.

[bouks]

Sorry but impossible to see the changes (14,154 additions, 14,154 deletions). I think we have to refuse the PR. Can you undo your commit and re-commit ?

[meh-uk]

I presume the error is formatting :).

[bchr02]

woops okay will do.

[flack]

@b2dac BTW: you've made the change in the the dist file. All files in this folder are generated by the build system. So even without the formatting issue, merging your change won't help, because it will get overwritten by the next run of grunt compile.

The best thing you could do would be to make the appropriate change in the source file in https://github.com/openpsa/jsgrid/tree/master/js (most likely, you'll need this file: https://github.com/openpsa/jsgrid/blob/master/js/grid.grouping.js). You'll need a development setup (see https://github.com/openpsa/jsgrid#development-setup) to build.

If that sounds to tedious, you can also send a pull against the dist file, we'll then copy the changes over to the source file after testing (but be in this case, the commit metadata from the PR will get lost, i.e. you won't be listed as a contributor in the github UI)

[meh-uk]

And being listed as a contributor looks good on your CV ;).

[bouks]

@flack i didn't see the dist :) @meh-uk Since i contribute jsgrid, i receive thousands job offers a day. :D

[meh-uk]

@bouks you're supposed to be encouraging him :p.

[flack]

I've been wondering if we should keep the dist folder in the repo anyhow. In the beginning, I added it because it was faster than writing a website and the code to put it on there. But now that we have gh-pages set up and running, we could also add another button to the download page where you can get an "all in one" pre-built zipball. That would be quicker than clicking all the checkboxes and waiting for download.js to finish all its tasks. We basically would only need the grunt task to create the zip file.

Then, we could add the dist folder to .gitignore, which would also keep all the "update build" commit noise out ot the repo. The only downside I can think of is that then people would have to look for (and find) the download link :-)

[bouks]

I was asking myself the same question this afternoon. Sure we don't need dist folder anymore. But for the all in one, what about default language ?

[flack]

@bouks Good question. Personally, I would say no default language, but that is because I use the grid mostly in multilingual sites, where one default language isn't going to help anyways. I would guess that most of the users (i.e. developers) will use only one language, and we know that they understand enough English to find the download link at least :-)

So I'm not sure. We could also add two minified files, jsgrid-en.min.js and jsgrid.min.js (or something along those lines). But adding one for each language would be overkill I think (and also slow to build and to download)

[meh-uk]

Including an English only one seems like it could be useful.

[bchr02]

Thanks @flack I have not used grunt prior to this. So this was a good learning experience for me. I appreciate the detailed explanation and assistance.

@meh-uk Yea it was a simple mistake with two of the variables. :wink: