edamontology / edam-browser

Stand-alone, lightweight, and fast JavaScript browser for EDAM and other ontologies
https://edamontology.github.io/edam-browser/
MIT License
14 stars 23 forks source link

Moving zoom controls to the tree map #68

Closed HagerDakroury closed 3 years ago

HagerDakroury commented 3 years ago

All the controls for the tree are currently placed under it, which seems a little confusing. (Aside from the grouping issue mentioned here https://github.com/edamontology/edam-browser/issues/63#issuecomment-810679880 )

An idea would be to move a few to the tree like in maps. Maybe add (+,-) for mousepads that don't support zooming in and out?

zoomnotonmap

Here are some examples:

FotoJet

bryan-brancotte commented 3 years ago

Hi @HagerDakroury

Thanks for creating a precise issue, your title is sharp and this are good ideas !

To just add another example from openstreetmap: Capture d’écran de 2021-04-01 11-11-35

bryan-brancotte commented 3 years ago

Solving it could be more work than what is generally needed for a "good first issue". Nevertheless I do not want to prevent anyone from trying to solve it, just keep in mind that it can consume time

HagerDakroury commented 3 years ago

Solving it could be more work than what is generally needed for a "good first issue". Nevertheless I do not want to prevent anyone from trying to solve it, just keep in mind that it can consume time

I believe so too. Plus I'm not sure if D3.js provides such an interface or it will need to be built from scratch

bryan-brancotte commented 3 years ago

I do not know either is d3.js provide something, an alternative solution would be base on css and placing html buttons over the d3js-tree-container

bryan-brancotte commented 3 years ago

Hi again @HagerDakroury

As you proposed this feature, and already did simpler task (#47) would you be interested in solving this issue ? I have though on how to solve it and there are ways to solve it in reasonable time. Keep me posted if you want the issue.

Best

Bryan

HagerDakroury commented 3 years ago

Hi @bryan-brancotte , yeah definitely! I'd like to work on this.

I'd need some time first to look for the best ways to do that design-wise and performance-wise.

Would also like to hear your thoughts and any guidance along the way!

bryan-brancotte commented 3 years ago

Ok, @HagerDakroury I've assigned you the issue !

I'll give you some time to look on your side of how you would do it, then ping me when you want us to talk about it (in this issue).

Good luck :)

HagerDakroury commented 3 years ago

So, @bryan-brancotte, After struggling for a while with d3 layouts, looks like they're pretty much standalone with no inherited controls (confirmed by looking at tree-reusable-d3.js code) I was looking for something like what most maps' APIs provide, but arrived at a dead-end here.

It's also not worth it to look for other layouts as the current tree layout does the job perfectly.

So, looks like your idea here is the way to go.

I do not know either is d3.js provide something, an alternative solution would be base on css and placing html buttons over the d3js-tree-container

How I see it, the current html layout is a div with 2 children one for the panel and one for the tree container. The panel (just the buttons, not the whole thing) will move to the tree container. screenshot

For the positioning, either: 1) Overlay the two components (tree and buttons ) with a relative position for the buttons 2) Use CSS grids for the whole thing (same results but may be more responsive)

Alternatively, we can keep the current html layout and manually change the positions of the buttons. However, I think grouping them with the tree is the logical way to go.

After the styling is done, zooming in and out using buttons can easily be added to the tree-reusable-d3.js code.

Is this approach optimal? I really wanted there to be another way but was stuck looking for one :smiley:

bryan-brancotte commented 3 years ago

Hi @HagerDakroury

Good analysis :smiley: , adding button inside the d3.js code is not easy. Note that you will find a (disabled) pop up inside the tree that is html floating inside, but I think that we should keep it as last option after every other have failed.

I agree with you that we should group them with the tree, the example at the beginning are perfect

In my mind I thought that putting all the button in a div that is before the tree (sibling) and have position: absolute;right:0px was a good option, but I'm opened to other approach (css grid?) as long as they allow us to have the nice look we see in the examples at the beginning of the issue.

Indeed zooming in/out is not available for now and have to be implemented, ideally similarly to browser.interactive_tree().cmd().collapseNotSelectedElement(), something like browser.interactive_tree().cmd().zoomIn() or browser.interactive_tree().cmd().zoom('in')

Here after a list of button we could have, feel free to discuss/improve/change:

I'll be less connected in until the 7th, but still do not hesitate to poke me.

keep up the good work :smiley:

HagerDakroury commented 3 years ago

Awesome!

Thanks for the heads-up about the button code!

I'll start with positioning the existing buttons and once I'm satisfied with a style, I'll start looking into implementing the zooming in/out.

Will keep you posted here along the way.

HagerDakroury commented 3 years ago

Hi @bryan-brancotte, I made a PR with an initial draft. I mentioned also that we may need to change the buttons labels to have a nicer overall style. I wasn't sure whether to label the PR as WIP or not. It technically works but may need further adjusting. So, let me know!

bryan-brancotte commented 3 years ago

:clap: