Netflix / flamescope

FlameScope is a visualization tool for exploring different time ranges as Flame Graphs.
Apache License 2.0
3k stars 168 forks source link

Enable Settings view for UI controls #42

Closed nhick closed 6 years ago

nhick commented 6 years ago

With more requests coming through for enhanced controls (#39 , #2 , #30 , ...), we should look at enabling the settings view or some other UI component and push some/all of the UI controls there that modify the view.

spiermar commented 6 years ago

I have a commented out modal window in the heatmap view. A simple button on top that opens a modal with the UI options for that view. Is that what you have in mind @nhick ?

nhick commented 6 years ago

Yes, that's perfect @spiermar . Created the issue to pr against and use it to discuss what to move into the settings view and what to leave out.

spiermar commented 6 years ago

Ok, so I made that assuming that one day we would have more options. The drawback of this solution is that it requires two user actions to get something done, opening the modal and then changing what's required, which can be a pain if you switch things a lot. Right now, I don't think we have too many options in the toolbar, enough to cause pain for users, so my suggestion would be to keep it that way until we reach this point, then migrate to the modal solution. Open to counter-arguments.

nhick commented 6 years ago

It's a fair point and it increases the cognitive load quite a bit (actually it's three hops/actions: open modal, select xyz, close modal). We can definitely keep everything flat in terms of info hierarchy and leave the controls the way they are; I personally prefer the immediate feedback, i.e. a user sees what happens right after they action one of the inputs. If we want to use a modal, I'd argue we should enable asap and not wait for a "breaking point" to migrate or just scratch it all together and revisit once we reach a point where it's not longer manageable for the user. Would you be open to scratching it altogether for now? If so, let's just close this for now.

spiermar commented 6 years ago

Yes, I agree.