Fix Table View group buttons #154 - #155
Conversation
* Replace colvisGroup jQuery “hide” code with simple call to new
“static get” variables.
Signed-off-by: John M. Horan <johnmhoran@gmail.com>
|
@johnmhoran We should avoid hard coding inverse groups of groups we have already defined. After reviewing I've discovered two problems:
However,
This can be fixed by replacing the static get method for example https://github.com/nexB/aboutcode-manager/blob/develop/assets/js/aboutCodeDataTables.js#L269 with The above needs to be outside of the class definition (I would put it below the class). |
|
Thanks @jdaguil . It would be very helpful to me if we could discuss your analysis so I can understand how I could have reached a similar conclusion and the details of how your solution works. |
* Static getters replaced with arrays defined outside the
AboutCodeDataTable class.
* .not selector replaced with .filter/.indexOf.
Signed-off-by: John M. Horan <johnmhoran@gmail.com>
|
@jdaguil @pombredanne Almost ready to push my latest commit, but it took me two attempts to successfully run the build script: |
|
The executable runs well, no errors. |
|
@johnmhoran we do not support building on gitbash on Windows. |
|
@pombredanne On Windows 10, I get similar behavior when using Git Bash to checkout the branch I want to build, opening the Windows command prompt, navigating to the project directory, and running |
|
@johnmhoran does it build on appevyor without errors? and is the appevyor build functional?
You seem to have some permissions problems on you machine that you need to resolve first IMHO |
|
@pombredanne Just checked my last commit result on AppVeyor (from yesterday afternoon) -- there are a series of npm warnings (2 re |
|
LGTM! |
@jdaguil Re the display problem with the Table View column-group buttons, the
showoption of the Buttons extension's colvisGroup button type is working properly, but for reasons I’ve not been able to pin down, thehideoption fails.As a result, when a user selects a column-group button, no columns are hidden. On initial load, all columns are displayed, which means that unless the user individually removes columns using the
Column visibilitybutton, clicking any of the 3 column-group buttons --Copyright info,License infoandOrigin info-- does not change the column display.These buttons last worked in Release 1.1.0-beta.3 (released 1/11/17). Our show/hide code was contained in
scancode.js(all of this inside$(document).ready(function)...:In the next release, Release v2.0.0-rc1: First v2.0.0 release candidate (released 4/21/17), the show/hide code was contained (as it is now) in
aboutCodeDataTables.js(all insideclass AboutCodeDataTable {...}):As noted above, the
showoption works while thehideoption does not work at all -- and commenting out the.notselector has no effect. The console shows no error.Curiously, if we adapt the
Release 1.1.0-beta.3code, the first line works, though the.notselector does not, resulting in the removal of all columns:==Alternative Solution==
I’ve done extensive research and tried a wide range of syntax variations using our current
colvisGroupjQueryhidecode as well as the earlier approach, thus far without success -- with one exception. (This approach could also be applied to issue #91Add a Package info group button to Table View, but I’ll defer tackling that issue until we’ve resolved this one.)Rather than starting with the full set of columns
AboutCodeDataTable.TABLE_COLUMNSand trying to exclude those columns we want to remain visible, I’ve modified several of our existingstatic getvariables and added several new ones -- includingNOT_COPYRIGHT_GROUP(),NOT_LICENSE_GROUP()andNOT_ORIGIN_GROUP(). Using these in a simplifiedhideoption codeblock achieves the desired functionality and does not appear to create any new problems. An example:This solution is a bit more complex (and less elegant) than the approach that worked in
Release 1.1.0-beta.3and it would not surprise me if there’s a way to adapt that pattern to our newclass AboutCodeDataTable {...}structure. I look forward to your thoughts and suggestions.