ManageIQ / manageiq

ManageIQ Open-Source Management Platform
https://manageiq.org
Apache License 2.0
1.35k stars 898 forks source link

Dynatree features that maybe aren't necessary #10431

Closed skateman closed 8 years ago

skateman commented 8 years ago

Hi!

There are some features implemented around dynatree that aren't so important, but in the same time it's hard to implement them with bootstrap-treeview.

1. Display host quadicon onMouseOver in a network tree There is no hover event in bootstrap-treeview.

2. Expand/collapse the whole tree with doubleclick There is no onDblcClick event in bootstrap-treeview.

3. Mark unsaved checkbox changes in C&U with blue There are 85 lines of ruby code rendering JS for this feature. It can be extracted into pure JS if necessary. Some of the trees with checkboxes have this feature, but some of them not, which is IMHO a bit inconsistent. screenshot from 2016-08-12 13-19-29

@dclarizio @epwinchell @martinpovolny @h-kataria @ZitaNemeckova

martinpovolny commented 8 years ago

Number 1. is not really useful I'd say and it does not scale. All the quadicons are rendered and hidden and on-hover they are shown. Only the rendering of the quadicons takes so long that in some cases the page seems broker (or actually is broken because the Apache times out on the appliance).

Moreover we can change this to on-click.

skateman commented 8 years ago

@martinpovolny onclick usually does something different, in this case redirects the browser to the current VM's summary page.

martinpovolny commented 8 years ago

Number 2. I'd disable. If I remember it correctly, @ZitaNemeckova was unifying the expand all behavior on trees converted to TreeBuilder in such a way that there always was a button to "Expand all".

martinpovolny commented 8 years ago

@martinpovolny onclick usually does something different, in this case redirects the browser to the current VM's summary page.

Then I'd just disable it or completely remove the view. It's not useful and it does not work if there are many VMs.

martinpovolny commented 8 years ago

Number 3. I don't understand. What type of changes is indicated by blue?

skateman commented 8 years ago

@martinpovolny you have a tree with checkboxes and if you change the state of the checkboxes, they will be shown with blue until they're saved...

epwinchell commented 8 years ago

1) I agree with @martinpovolny Number 1 always seemed hokey to me and isn't really in keeping with Patternfly or the rest of the UI and isn't responsive. @dclarizio @serenamarie125 Thoughts?

martinpovolny commented 8 years ago

I see. That would require some custom javascript to change the style. Isn't the fact that the form was manipulated demonstrated by the form buttons (enabled/disabled?)

If so, I would remove this feature. At least for now.

skateman commented 8 years ago

It can be implemented later in the external JS.

ZitaNemeckova commented 8 years ago

@martinpovolny Expand all button is not always there (just with trees that had it before) but could be easily added.

dclarizio commented 8 years ago

@skateman

1 . Display host quadicon onMouseOver in a network tree There is no hover event in bootstrap-treeview.

We can drop this feature . . . if anyone asks for it back, we will implement it differently.

2 . Expand/collapse the whole tree with doubleclick There is no onDblcClick event in bootstrap-treeview.

Agree with others that an expand all checkbox can be added for trees that need it.

3 . Mark unsaved checkbox changes in C&U with blue There are 85 lines of ruby code rendering JS for this feature. It can be extracted into pure JS if necessary. Some of the trees with checkboxes have this feature, but some of them not, which is IMHO a bit inconsistent.

This one IS NEEDED in the areas it is used and should NOT be dropped. It is VERY confusing if we don't mark the changed nodes in the trees that allow you to set a bunch of tags or choose a bunch of Hosts and/or Folders. The user needs to be confident of which items were touched.

I would think a pure JS solution would be to simply change the text color when the node's checkbox is clicked: if black turn it blue, if blue turn it black. It should follow then that any node not set to it's initial value will be blue. Of course, if the server redraws the tree during edit, this solution won't work. I leave that up to you.

skateman commented 8 years ago

@priley I think this third one should go directly into bootstrap-treeview, what do you think? @dclarizio we can do something like angular's dirty/pristine to overcome the issues with re-rendering.

skateman commented 8 years ago

@dclarizio something like this https://github.com/jonmiles/bootstrap-treeview/pull/273

priley86 commented 8 years ago

@skateman I agree ;)