monarch-initiative / phenogrid

The phenogrid widget
13 stars 14 forks source link

Odd behavior when loading multiple instances of Phenogrid on the same page #226

Closed yuanzhou closed 8 years ago

yuanzhou commented 8 years ago

Jules noticed this issue when integrating multiple instances of Phenogrid into the IMPC site.

I'm loading multiple instances of the grid on the page (once per disease/gene panel) and there is some horrible global variable type issues. I suspect this is because of the JQuery selections for various elements having the same name. For example the title gets garbled with multiple renderings overwriting each other, mousing-over a HP-MP node on a lover grid results in a pop-up over the same cell in the top-most grid, but displays the data from the cell the mouse is hovering over (see attached where the mouse is hovering over the node 'Hypertelorism-Mmp2') . Another example is opening a new grid in a panel below an existing one - only the menu will show in the new panel.

phenogridinimpcissues

yuanzhou commented 8 years ago

I foresee there are several things involved to fully address this issue:

For the specific needs of Phenogrid in IMPC, is there any UI elemetns of control features we can remove form Phenogrid in this case? @harryhoch

yuanzhou commented 8 years ago

From @harryhoch:

as far as multiple instances go, I have a suggestion for something that might work. Currently, each phenogrid element is given an ID, which must be unique on the page, correct? What if we replaced the ID with a pair of classes? One class would replace the current ID with a class name - thus, #phen_vis become .phen_vis. The second class would be a unique ID associated with each instance of phenogrid on a page. That way, something like $(“.phen_vis .pg1234”) would select the phen_vis container specifically associated with phenogrid instance pg1234, etc. Would that work? It might be a bit of a pain to implement, but I think it might be conceptually possible.

julesjacobsen commented 8 years ago

The phenogrid constructor requires a parent element in which the phenogrid is inserted. Using the id of this parent element and appending this to the phenogrid element ids would make the id unique, providing the parent element id is unique. If it isn't then we're no worse off than we are currently.

yuanzhou commented 8 years ago

@julesjacobsen exactly, I've been making changes using this approach. And also changed the corresponding CSS IDs into classes, and added additional CSS classes to recreate the styling since we won't be able to specify the styling based on the dynamic containing div's ID.

yuanzhou commented 8 years ago

This issue has been addressed in PR #230