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

Refactor: Update ontology viewer from old JS to Turbo + Stimulus #223

Conversation

syphax-bouazzouni
Copy link
Contributor

@syphax-bouazzouni syphax-bouazzouni commented Sep 21, 2022

Prerequisite

What and why

This PR simplifies and updates our current JS code, by removing 814 lines of JS code but still keeping the same behaviors and features.

It's possible using the two following tools:

  • Tubro frame: will use Ajax to send and replace the HTML in a specified part of the current page (without reloading and JS code), this will replace most parts of our current JS code that are exactly doing that.
  • Stimulus JS: will centralize/isolate js code, so that we can reuse it more easily and avoid duplication and unused/forgotten code (a problem that we have a lot currently).

After the PR

Enregistrement.de.l.ecran.2022-09-21.a.22.34.01.mov

Changes

  • Replace bp_ontology_viewer.js with turbo frame in the ontology viewer ( same behavior with 417 (JS) lines less)(9371bc4)

  • Fix visits chart not showing( (see The visits chart is not displayed in the summary page #179) f73f7fc)

    • replacing app/assets/javascripts/ontologies.js with a stimulus controller "load-chart"
  • Fix more... submission button that was not working (4e06e29)

    • clicking in the 'more...' will show the full table of submissions and replace the "more..." text with "less..." (and vis-versa)
  • Remove empty functions in app/assets/javascripts/bp_notes.js (10 lines less) (0987e08)

  • Fix notes unsubscribe button (d21c570)

  • Replace concepts tree view node click js code with turbo_frames (same behavior with 259 (JS) lines less)

  • Update and centralize Split dependency (17 JS code less) (a8e029b)

    • install split with yarn (js package manager) and remove lines that upload it <script src="//unpkg.com/split.js/dist/split.min.js">
    • centralize the split usage inside a stimulus controller called "container-splitter"
  • Centralize Simple tree dependency and replace JS code that displays the treeview with a turbo frame (41 JS code less) (675c4f3)

    • remove/move the file app/assets/javascripts/bp_class_tree.js.erb (82 lines of js code)
    • centralize the simple tree view usage inside a stimulus controller called "simple-tree"
    • use a turbo_frame in app/views/ontologies/visualize.html.haml to display the tree view
    • remove unused function in app/assets/javascripts/bioportal.js.erb (16 lines less)
  • Extract/centralize autocomplete behavior in a stimulus controller (ecfa8ca)

    • remove/move the code that does the jump to a concept feature from app/assets/javascripts/bp_visualize.js.erb to a stimulus controller (87 lines of JS code)
  • Replace the js code responsible for showing concepts tabs with bootstrap tabs ( 70 lines js code less )(6910d78)

@jvendetti
Copy link
Member

Hi @syphax-bouazzouni - thanks for the pull request. I'm interested in moving forward on this one, but I realize that a fair amount of time has gone by since it was submitted. Is your AgroPortal code base still relatively in alignment with the changes you made here?

@jvendetti
Copy link
Member

I also wonder if you'd be willing to drop the commits from this pull request from May 25 - 28 that perform the Rails 7 upgrade?

@syphax-bouazzouni syphax-bouazzouni marked this pull request as draft April 17, 2024 23:38
@jvendetti
Copy link
Member

jvendetti commented May 1, 2024

I'm testing this pull request in my local dev environment. I've run into a variety of issues, so I'm building a list here for what I need to fix before I can merge this into our master branch.

  • Navigating to any ontology throws an undefined method escape error
  • Navigating to any ontology throws a No route matches error for a fetch_results action
  • The top-level tabs on the ontology landing pages don't work properly unless you click on the Summary tab. After the first click on the Summary tab, then they work as expected. If you don't click the Summary tab, clicking on any of the other tabs doesn't do anything.
  • Navigating to any ontology marked as flat (e.g. MEDDRA) results in a 500 internal server error, for URLs like this: http://localhost:3000/ajax/classes/treeview?concept=bp_fake_root&ontology=MEDDRA
  • The Classes pane for flat ontologies (e.g. MEDDRA) perpetually displays a loading message underneath the Jump to text area
  • The top-level Widgets tabs for all ontologies is blank
  • For all ontologies, only the Details sub-tab works. All others (Visualization, Notes, Class Mappings) don't respond to clicks - this is Bootstrap-related.
  • The autocomplete functionality in the Jump to text are is broken. Error: TypeError: jQuery(...).bioportal_autocomplete is not a function
  • The Visualization tab for all classes is broken - perpetually displays a loading message
  • New Term Requests tab doesn't show any content

@syphax-bouazzouni
Copy link
Contributor Author

@Bilelkihal can you do the follow-up of this?

@Bilelkihal
Copy link

@Bilelkihal can you do the follow-up of this?

Okay

@jvendetti
Copy link
Member

Mutually agreed with submitter to close - too much time went by between original submission date and initial attempt to merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants