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

Bundle chira #61

Merged

Conversation

OlegZharkov
Copy link

Introduce webpack to Chira Visualization, remove all dependencies from git repo (jquery, underscore, bootstrap, etc...)
It would be nice, if someone familiar with this visualization will try it locally before the actual merge. You don't need to rebuild the while client, yarn run gulp plugins will work

@bgruening
Copy link
Member

@anuprulez @pavanvidem can you please test? If this is in place we could upstream the changes with the datatypes

@anuprulez
Copy link
Member

anuprulez commented Mar 24, 2020

I tested it and found the following issues:

  • If there is an alignment present, it shows an error:

viz_alignment_2

An example on the EU server
viz_1

I think visualize-alignment.js file is not bundled

  • The left and right margin are too big (the margins between viz and history and tool panels).

margin_1

An example on the EU server
margin_1_1

  • it does not take the full screen

expand_3_1

An example on the EU server
expand_3

  • The dropdowns are not taking the maximum width of their items

dropdowns_4

An example on the EU server

dropdowns_4_1

Thanks a lot for working on this @OlegZharkov
Would you like to have a look at it? @pavanvidem
(if yes, then please follow these steps;

  1. Install gulp
  2. Execute yarn run gulp plugins at $galaxy/client )

ping @bgruening

@OlegZharkov
Copy link
Author

Oh, I see. How do I actually see an alignment? I pressed a few buttons and it always showed that alignment is not there. But anyway, I'm pretty sure why visualize-alignment.js isn't there, I'll fix it. As for margin, I have an idea, I'll try it tomorrow.

Thanks, a lot @anuprulez for the feedback. Visualize would be there very very soon... for margin I will need to investigate a bit. From Screenshots it looks like a problem with font-awesome. I'll just change a few things... Thanks!

@bgruening
Copy link
Member

bgruening commented Mar 24, 2020

@anuprulez;

(if yes, then please follow these steps;

I think, hope, make client-production should work. So that we can deploy this easily. Could you test this with make client-production as well?

@OlegZharkov
Copy link
Author

@bgruening make client-production must definitely work, that why this line is there . yarn run gulp plugins is only the case, if you don't want to rebuild the whole client. Even single run.sh should do the job actually (compile visualization and copy bundled stuff to static dir).

@anuprulez you don't need to install gulp globally. Just go to galaxy/client and yarn install && yarn run gulp plugins. I want to check the use case with alignment. How do I see one?

@anuprulez
Copy link
Member

@OlegZharkov Please follow these steps:

  1. On the first page, please click on any bar to go to the next page
  2. On the second page, please choose Hybrid from --filter-- dropdown, <> from --operator-- and write NA in the Enter value textbox
  3. Press enter

You will see only those interactions on the left with alignment information. Please click on +
icon and select a row. You will find the alignment.

Example:
ex

@OlegZharkov
Copy link
Author

OlegZharkov commented Mar 24, 2020

@anuprulez Thanks a lot! That was very very useful! Should work now. Margin is still in the process of debugging...

@anuprulez
Copy link
Member

@OlegZharkov I tested it again. Looks ok to me 👍

@OlegZharkov
Copy link
Author

@anuprulez Thanks a lot for testing!The problem was css loading race. Well, since it works now... @bgruening should I create another PR on upstream repo?

@bgruening
Copy link
Member

@bgruening should I create another PR on upstream repo?

this is a question for @anuprulez ... I think this and the datatype should go upstream. Hopefully, this will be accepted. If Anup does not care about the commit history and the credits you are welcome to do the PR. But maybe Anup wants to do it.

@bgruening bgruening merged commit d81645c into usegalaxy-eu:release_19.09_europe Mar 24, 2020
@pavanvidem
Copy link

thanks a lot all for your efforts!

@OlegZharkov
Copy link
Author

@bgruening should I create another PR on upstream repo?

this is a question for @anuprulez ... I think this and the datatype should go upstream. Hopefully, this will be accepted. If Anup does not care about the commit history and the credits you are welcome to do the PR. But maybe Anup wants to do it.

Commit history is preserved with cherry pick, isn't it? But I don't really mind, @anuprulez you're always welcome to rebase it ;)

@anuprulez
Copy link
Member

@OlegZharkov if you like, please create a PR on Galaxy main. The review comments we can share if you like or I can take them. Thanks a lot for working on 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

Successfully merging this pull request may close these issues.

None yet

4 participants