GMOD / Apollo

Genome annotation editor with a Java Server backend and a Javascript client that runs in a web browser as a JBrowse plugin.
http://genomearchitect.readthedocs.io/
Other
128 stars 85 forks source link

Add select-all / unselect-all to TrackPanel #1724

Closed hsiaoyi0504 closed 7 years ago

hsiaoyi0504 commented 7 years ago

It's the same as bhofmei/jbplugin-hierarchicalcheckbox#3, because it is related to both repos.

When I tested on JBrowse 1.12.3, it works with this hierarchical check box plugin , but when I used the Apollo 2.0.6 (also with JBrowse 1.12.3 and it's automatically downloaded by Apollo. Plugin installation is by specifying setting in apollo-config.groovy), it didn't work (no checkboxes show up).

I simply uncomment the line console.log("plugin HierarchicalCheckboxPlugin constructor"); in main.js of hierarchical check box plugin and it shows in console, so the plugin seems to be loaded, but for some reason it didn't work.

nathandunn commented 7 years ago

You should be able to select "JBrowse Track Selected" in the tracks menu if you want to see it in 2.0.6:

screen shot 2017-08-09 at 2 57 56 pm

On the master branch (2.0.7-SNAPSHOT) that is possible as well.

Additionally, you can supply categories and it should pick them up. I think there is a limit to how much it categorizes so it may not work for you, but you won't have to use a separate plugin / browser that way (though @bhofmei writes good code so there are probably a few other advantes):

screen shot 2017-08-09 at 3 00 06 pm

If this is something you still need, add screenshots and console logs here.

FYI @childers

hsiaoyi0504 commented 7 years ago

To clarify, what I want is the ability to “select / unselect all” within a category. The preview of this plugin can be seen in following figure. I can correctly see this in JBrowse, but can't make it work on Apollo. image

The line I uncomment is here in constructor of Hierarchical Checkbox Plugin.

nathandunn commented 7 years ago

@hsiaoyi0504

You'll want to edit the TrackPanel:

https://github.com/GMOD/Apollo/blob/75c1d24a01bb7eed3ff83596b94261afcdac49eb/src/gwt/org/bbop/apollo/gwt/client/TrackPanel.java#L200

and here:

https://github.com/GMOD/Apollo/blob/75c1d24a01bb7eed3ff83596b94261afcdac49eb/src/gwt/org/bbop/apollo/gwt/client/TrackPanel.java#L256

This is just GWT code.

Handling the select / unselect is here:

https://github.com/GMOD/Apollo/blob/75c1d24a01bb7eed3ff83596b94261afcdac49eb/src/gwt/org/bbop/apollo/gwt/client/TrackPanel.java#L226

You'll want to do most of this in devmode with GWT.

I typically use ./apollo devmode (but you have to run ./apollo run-local first.

More references: http://www.gwtproject.org/ and this bootstrap binding: http://gwtbootstrap3.github.io/gwtbootstrap3-demo/

nathandunn commented 7 years ago

Let me know if you run into any issues. Keep in mind that devmode launches the gwt dev process which needs to be killed externally (you can look at how to run it from the shell script).

childers commented 7 years ago

@nathandunn Thanks for all the great pointers!

nathandunn commented 7 years ago

Sure. Don't hesitate to ask if you get stuck. It's a bit different development environment. But to see changes you should just reload the page if running Dev mode

Nathan

On Aug 10, 2017, at 11:57 AM, childers notifications@github.com wrote:

@nathandunn Thanks for all the great pointers!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

hsiaoyi0504 commented 7 years ago

Hi @nathandunn , thanks for helping us.

I am working on implementing this feature, but I don't know how to configure javac options and can't find related helps in the documentation. For example, followings are messages I got when I run ./apollo run-local. I would like to use such options like -Xlint:unchecked, --stacktrace, --info, and --debug. Can you help me to configure these?

Installing Perl prerequisites ...Prerequisites installed, finished.
:javac
Note: /Users/hsiaoyi/Documents/workspace/NAL-i5K-Apollo/src/gwt/org/bbop/apollo/gwt/client/TrackPanel.java uses or overrides a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.
:gwtc FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':gwtc'.
> Java returned: 1

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output.

BUILD FAILED

Total time: 4 mins 42.268 secs

Thank you in advance

nathandunn commented 7 years ago

A couple of things to keep in mind:

1 - We just merged changes redoing a lot of the TrackPanel (some functionality, just replaced the slider widget due to performance problems).

2 - what version of javac do you have: javac -version?

3 - what happens if you type ant gwtc?

hsiaoyi0504 commented 7 years ago

Hi @nathandunn So for 1, you suggest we should not implement this feature at this time point?

For 2, my javac is version 1.8.0_112.

For 3, I didn't install ant before, but after I installed and type that. I think I get what I want (some more specific error messages) through this.

Buildfile: /Users/hsiaoyi/Documents/workspace/NAL-i5K-Apollo/build.xml

javac:

gwtc:
     [java] Compiling module org.bbop.apollo.gwt.Annotator
     [java]    Ignored 1 unit with compilation errors in first pass.
     [java] Compile with -strict or with -logLevel set to TRACE or DEBUG to see all errors.
     [java]    Computing all possible rebind results for 'org.bbop.apollo.gwt.client.MainPanel.MainPanelUiBinder'
     [java]       Rebinding org.bbop.apollo.gwt.client.MainPanel.MainPanelUiBinder
     [java]          Checking rule <generate-with class='com.google.gwt.user.rebind.rpc.ServiceInterfaceProxyGenerator'/>
     [java]             [ERROR] Hint: Check the inheritance chain from your module; it may not be inheriting a required module or a module may not be adding its source path entries properly
     [java]    [ERROR] Errors in 'org/bbop/apollo/gwt/client/MainPanel.java'
     [java]       [ERROR] Line 54: Failed to resolve 'org.bbop.apollo.gwt.client.MainPanel.MainPanelUiBinder' via deferred binding
     [java]    Tracing compile failure path for type 'org.bbop.apollo.gwt.client.TrackPanel'
     [java]       [ERROR] Errors in 'file:/Users/hsiaoyi/Documents/workspace/NAL-i5K-Apollo/src/gwt/org/bbop/apollo/gwt/client/TrackPanel.java'
     [java]          [ERROR] Line 376: Cannot refer to the non-final local variable trackBodyList defined in an enclosing scope
     [java]    [ERROR] Hint: Check the inheritance chain from your module; it may not be inheriting a required module or a module may not be adding its source path entries properly

BUILD FAILED
/Users/hsiaoyi/Documents/workspace/NAL-i5K-Apollo/build.xml:35: Java returned: 1

Total time: 8 seconds

However, I would like to know how to customize the javac options in this project? Or options for ant is what I am seeking for? I have tried ant gwtc -debug, but I think it can be more specific through options like -strict, or -logLevel. Sorry for that I didn't use ant before.....

nathandunn commented 7 years ago

For 1. Yes, you can still implement the feature. You'll just have to repull the code from master, that's all. The change went in earlier this week.

If you look in the build.xml file under the gwtc target, you can see the different options there.

If you use the devmode target (just one) after you do the compilation using gwtc and you are running apollo in a different terminal (./apollo run-app) you should only need to refresh your browser and the recompilation will just happen. You should see a red application come up when you use devmode. You'll only want one of these. This is much quicker than compiling each time (well, once you have the initial bugs fixed).

Also, you can check for some of those bugs by just running ./apollo compile as well.

More info here: http://www.gwtproject.org/doc/latest/DevGuideCompilingAndDebugging.html#dev_mode

hsiaoyi0504 commented 7 years ago

Hi @nathandunn, I have two questions. Maybe they are stupid, but it would be an useful help if you can help me these.

I see there is a checkBoxButon in each panelBody. https://github.com/GMOD/Apollo/blob/4c1a65f54a30e72b6c2e07b2d1bf0d2583afff44/src/gwt/org/bbop/apollo/gwt/client/TrackPanel.java#L213

First, these panelBody are generated and added to panelCollapse. https://github.com/GMOD/Apollo/blob/4c1a65f54a30e72b6c2e07b2d1bf0d2583afff44/src/gwt/org/bbop/apollo/gwt/client/TrackPanel.java#L351

I would need to update the Boolean value of these selected checkBoxButton. How can I access these checkBoxButton? Can I access them through panelCollapse or I should create a list and save these panelBody to make me accessible to these?

Second, if I can directly called the value change handler, or I should just update the value through setValue()? https://github.com/GMOD/Apollo/blob/4c1a65f54a30e72b6c2e07b2d1bf0d2583afff44/src/gwt/org/bbop/apollo/gwt/client/TrackPanel.java#L227

nathandunn commented 7 years ago

These are good questions.

For the first question, I think the best way to do what you are trying to do is to create a map between TrackInfo and the CheckBoxButton (you can see at the top of TrackPanel.java) that I do that for other things. For example if you look at categoryMap. Another possibility would be to use a simple Composition to create an object that uses both TrackInfo and CheckBoxButton in the same object and then replace categoryMap to use that object.

For your second question, I think there are two ways to solve this problem.

  1. refactor the contents of the addValueChangeHandler() to a new method (lets call it "handleSelect()"). When you call "unselect / select all" it calls a method called "selectValue(TrackInfo trackInfo,boolean newValue)" This would call trackInfo.setValue(newValue) and then handleSelect(trackInfo,newValue).
  2. you can make a direct all on the dom for each I think. This is fine for a backup, but I don't think its as clean: https://stackoverflow.com/a/1238512/1739366
nathandunn commented 7 years ago

Also, feel free to convert anything to a lambda if you want: https://docs.oracle.com/javase/tutorial/java/javaOO/lambdaexpressions.html

We did this all under Java 7, initially (so no lambdas). Certainly not necessary, though.

nathandunn commented 7 years ago

BTW, you should be able to add the select / unselect all button directly to the PanelHeader. Probably before the heading or after the count badge.

hsiaoyi0504 commented 7 years ago

Hi @nathandunn,

Thank you for helping me these. But I don't understand that why the handleSelect function needs to have input the trackInfo. If I have a map between TrackInfo and the CheckBoxButton, let me called it trackMap. I can iterate over trackInfoList to make things update. That it, what I think is

panelSelect.addValueChangeHandler(new ValueChangeHandler<Boolean>() {
    @Override
    public void onValueChange(ValueChangeEvent<Boolean> event) {
        value = panelSelect.getValue();
        for (TrackInfo trackInfo : trackInfoList) {
            trackInfo.setValue(value);
            trackMap.get(trackInfo).handleSelect(value);
        }
    }
});

where panelSelect is the select all/unselect all CheckBoxButton

nathandunn commented 7 years ago

You might be right. That was just my quick thought on it, but I haven’t spent much time trying to dig in a solution.

If this works for you, then it is probably correct.

On Aug 23, 2017, at 9:38 AM, hsiao yi notifications@github.com wrote:

Hi @nathandunn https://github.com/nathandunn,

Thank you for helping me these. But I don't understand that why the handleSelect function needs to have input the trackInfo. If I have a map between TrackInfo and the CheckBoxButton, let me called it trackMap. I can iterate over trackInfoList to make things update. That it, what I think is

panelSelect.addValueChangeHandler(new ValueChangeHandler() { @Override public void onValueChange(ValueChangeEvent event) { value = panelSelect.getValue(); for (TrackInfo trackInfo : trackInfoList) { trackInfo.setValue(value); trackMap.get(trackInfo).handleSelect(value); } } }); where panelSelect is the select all/unselect all CheckBoxButton

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GMOD/Apollo/issues/1724#issuecomment-324392634, or mute the thread https://github.com/notifications/unsubscribe-auth/AAt2qjJAQiHZ9aqxSIsHY3wDeX3cqWyaks5sbFWAgaJpZM4Oxjv0.

hsiaoyi0504 commented 7 years ago

Hi @nathandunn

Although it still needs to modify some style sheet, I think I successfully implemented what we want. Thank you for great helps! Can I make a PR? Or you think it should not present in current Apollo, and we should just make these modifications in our owned Apollo.

nathandunn commented 7 years ago

Yes please make a pr! Thanks.

Nathan

On Aug 23, 2017, at 11:36 AM, hsiao yi notifications@github.com wrote:

Hi @nathandunn

Although it still needs to modify some style sheet, I think I successfully implemented what we want. Thank you for great helps! Can I make a PR? Or you think it should not present in current Apollo, and we should just make these modifications in our owned Apollo.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

hsiaoyi0504 commented 7 years ago

Hi @nathandunn

I created the PR #1733. However, with several attempts, I still can't modify the stylesheet of the CheckBoxButton. I tried edit the css in Apollo/src/gwt/org/bbop/apollo/gwt/public/theme.css, but it seems that the stylesheet is overwritten by application.css. However, it doesn't make sense to change css of overall application. Maybe I have misunderstood something?

nathandunn commented 7 years ago

I’ve had to use the !important flag in some cases. While not ideal, if you are only using it for designated classes it should be fine.

For example:

https://github.com/GMOD/Apollo/blob/master/src/gwt/org/bbop/apollo/gwt/public/theme.css#L57-L60 https://github.com/GMOD/Apollo/blob/master/src/gwt/org/bbop/apollo/gwt/public/theme.css#L57-L60

If you have the css class defined, I can mess with it when I test the PR, as well.

I’ll take a look at it tomorrow (about 15 hours from now).

On Aug 23, 2017, at 8:52 PM, hsiao yi notifications@github.com wrote:

Hi @nathandunn https://github.com/nathandunn I created the PR #1733 https://github.com/GMOD/Apollo/pull/1733. However, with several attempts, I still can't modify the stylesheet of the CheckBoxButton. I tried edit the css in Apollo/src/gwt/org/bbop/apollo/gwt/public/theme.css, but it seems that the stylesheet is overwritten by application.css. However, it doesn't make sense to change css of overall application. Maybe I have misunderstood something?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GMOD/Apollo/issues/1724#issuecomment-324525690, or mute the thread https://github.com/notifications/unsubscribe-auth/AAt2qhqHI7PiAKI6YqkPzbQ9UHriw1Tvks5sbPN5gaJpZM4Oxjv0.

hsiaoyi0504 commented 7 years ago

Hi, @nathandunn Thanks for helping ! I would try a little bit more and fix anything I can do. I see that there are some improvements suggested by codacy.