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

Add control to reset local view state #2080

Merged
merged 2 commits into from
Jan 10, 2017
Merged

Conversation

jpellizzari
Copy link
Contributor

Fix for #1872

Added a little 'trash can' icon on the help menu that will clear the 'scopeViewState' key in localStorage. It is possible to get into a loop where you are in an invalid view state because refreshing the page reads from localStorage. Clicking the trash can then refreshing should load a clean UI.

screen shot 2016-12-09 at 2 30 59 pm

@jpellizzari jpellizzari requested a review from bowenli December 9, 2016 22:47
@jpellizzari jpellizzari self-assigned this Dec 9, 2016
@bowenli
Copy link
Contributor

bowenli commented Dec 10, 2016

appears slightly outside the modal when i run it:
screen shot 2016-12-09 at 16 01 56

@rade
Copy link
Member

rade commented Dec 12, 2016

I have run into #1872 too, so we should not assume that this is exceedingly rate.

Few user encountering this will think "hang on, let me look at the help menu for an obscure trash can icon hidden in a corner". More so since everything else in the help menu concerns keyboard controls and search syntax.

IMO @davkal's original suggestion from #1872, before he changed his mind, of adding an icon to the icon group in the bottom right corner makes more sense. Several of these icons already are "rendering logic controls". Yes, I know that corner is getting busy, but that's a separate issue.

The labelling of the button, and iconography, also needs some more thought. "Clear UI state" does not mean anything to a user. "Reset View"? And perhaps use Reset Button as the icon.

@davkal
Copy link
Contributor

davkal commented Dec 12, 2016

  • I discourage the use of an icon here and prefer a clearly labeled button "Reset User Interface"
  • I would put it in the help overlay for 2 reasons: the button will be used very little (we should track its usage) and users will have seen it there already while they explored the UI
  • the button should have an explanation above it containing typical error descriptions that clicking it would solve, as well as assurances that nothing gets deleted (apart from the UI state)
  • to give another entrypoint, we could add a new overlay that appears when the bug icon is clicked. That overlay then has the usual button to link to the bug tracker, and could have a preamble section giving users hints on trying to reset the UI first, with a reset button

@rade
Copy link
Member

rade commented Dec 12, 2016

I discourage the use of an icon here and prefer a clearly labeled button "Reset User Interface"

Why? We already have seven icons in the bottom right corner!

I would put it in the help overlay for 2 reasons: the button will be used very little (we should track its usage) and users will have seen it there already while they explored the UI

I really don't think anybody will look for the Reset in the Help screen. Even if they have been there before. That's because everything else (i.e. 99%) of the help screen is passive, info only.

the button should have an explanation above it containing typical error descriptions that clicking it would solve, as well as assurances that nothing gets deleted (apart from the UI state)

Other icons provide such info in their bubbles, e.g. "Force re-layout (might reduce edge crossings, but might shift nodes around)"

to give another entrypoint, we could add a new overlay that appears when the bug icon is clicked.

That seems like the best option to me. Then we could also move: "Save Canvas" and "Save Raw Data" to that overlay, since they are mainly useful for debugging.

@davkal
Copy link
Contributor

davkal commented Dec 12, 2016

[Label instead of icon] Why? [and ] Other icons provide such info in their bubbles, e.g. "Force re-layout (might reduce edge crossings, but might shift nodes around)"

It's a destructive action and should be clearly labeled in what it does. It potentially triggers a page reload which is disorienting if clicked accidentally (therefor the tooltip will be of no help).

We already have seven icons in the bottom right corner!

I dont understand this argument.

I really don't think anybody will look for the Reset in the Help screen. Even if they have been there before. That's because everything else (i.e. 99%) of the help screen is passive, info only.

This could be answered by user research.

That seems like the best option to me. Then we could also move: "Save Canvas" and "Save Raw Data" to that overlay, since they are mainly useful for debugging.

Very much agree. Those could be grouped together and I think it's not too much work to include this in this PR.

@rade
Copy link
Member

rade commented Dec 12, 2016

This could be answered by user research.

I can give you one data point: me :)

@jpellizzari jpellizzari changed the title Add control to reset local view state [WIP] Add control to reset local view state Dec 16, 2016
@jpellizzari jpellizzari force-pushed the 1872-clear-ui-state branch 4 times, most recently from 74bd73c to 2b4bf01 Compare January 4, 2017 23:51
@jpellizzari
Copy link
Contributor Author

I have made a change to add a 'Debugging/Troubleshooting' to to contain related buttons:
screen shot 2017-01-04 at 3 35 23 pm

Note that the 'Save raw data as json' button has been removed from the footer and placed in the new dialog.

Feedback is welcome; @davkal could you review the code when you get a chance?

@jpellizzari jpellizzari requested a review from davkal January 4, 2017 23:56
@jpellizzari jpellizzari assigned davkal and unassigned jpellizzari Jan 4, 2017
@jpellizzari jpellizzari changed the title [WIP] Add control to reset local view state Add control to reset local view state Jan 5, 2017
@bowenli
Copy link
Contributor

bowenli commented Jan 5, 2017

The power symbol (circle broken by line) is an IEEE/IEC standard (https://en.wikipedia.org/wiki/Power_symbol), which would be not appropriate to use here.

Can we use something like fa-undo for the reset state? screen shot 2017-01-04 at 16 29 25

@rade
Copy link
Member

rade commented Jan 5, 2017

As per #2080 (comment), the "Save Canvas" button should be moved into this new dialog too.

@jpellizzari
Copy link
Contributor Author

@bowenli I do not recognize the authority of the IEEE, but I do think that icon makes more sense, since it reloads the page.

@jpellizzari
Copy link
Contributor Author

Updated with feedback:
screen shot 2017-01-05 at 11 32 25 am

@rade
Copy link
Member

rade commented Jan 5, 2017

nice

@davkal
Copy link
Contributor

davkal commented Jan 9, 2017

Works great and code LGTM!

UI consistency nit: this introduces a new styling. Although the underlining invites clicking, I would have chosen a more button-y approach based on an existing element, like the view mode selector (mostly because it's a button and has an icon):

screen shot 2017-01-09 at 17 17 48

with the click state as seen left, and the neutral state as seen right.

Or we use this opportunity to import a ui-components button!

@jpellizzari
Copy link
Contributor Author

@davkal I added a mouseover effect to keep the styling more consistent.
screen shot 2017-01-09 at 6 11 14 pm
In general, underlined links in scope are navigation actions, which is what I would classify these actions as. There is no shared state between the buttons, so an active/inactive paradigm doesn't make sense to me.

I tried this with a more button-y approach, and it didn't add any meaning and looked weird:
screen shot 2017-01-09 at 5 56 06 pm

@jpellizzari jpellizzari merged commit 4d50c69 into master Jan 10, 2017
@jpellizzari jpellizzari deleted the 1872-clear-ui-state branch January 17, 2017 06:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants