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

Improve open imagery "Import" button UX #202

Closed
lamby opened this Issue Nov 2, 2017 · 10 comments

Comments

Projects
None yet
4 participants
@lamby
Copy link
Contributor

lamby commented Nov 2, 2017

Current Behavior

Pressing the Import button expands the filename browser. This makes sense. However, pressing Import again contracts the browser, which does not make much sense as you

Expected Behavior

Unsure about the ideal behaviour. One possibility at this point is that the copy changes after it has expanded (eg. "hide import"). Alternatively, we could hide the button altogether. Or we explicitly change it to "Show files / hide files". Please experiment!

Checklist before submitting

  • I have confirmed this using the officially supported Docker Compose setup using the local.yml configuration and ensured that I built the containers again and they reflect the most recent version of the project at the HEAD commit on the master branch
  • I have searched through the other currently open issues and am confident this is not a duplicate of an existing bug
  • I provided a minimal code snippet or list of steps that reproduces the bug.
  • I provided screenshots where appropriate
  • I filled out all the relevant sections of this template
@louisgv

This comment has been minimized.

Copy link
Contributor

louisgv commented Nov 2, 2017

If the app allows user to import multiple image, then we should remove the call to contracts the browser from the import button event handler, and instead add it to a drop-down button element similar to what we have with nodule.

@lamby lamby added the official label Nov 2, 2017

@Serhiy-Shekhovtsov

This comment has been minimized.

Copy link
Contributor

Serhiy-Shekhovtsov commented Nov 18, 2017

I am not sure if we should allow selecting multiple images. In documentation we can find the following:

Allow user to select a given image and start a new “Case.” This image will be used in the identification step next

Also it will complicate some things, including this story: Prevent progress to next stage of analysis until "completed" the current step (#204).
Please, let me know what do you think? (This issue is blocking me with #145)

@louisgv

This comment has been minimized.

Copy link
Contributor

louisgv commented Nov 18, 2017

@Serhiy-Shekhovtsov After reading the requirement again, I think we should separate the two function into 2 UI elements (button), one for importing image and one for expanding the image. So maybe for #145, you can simply change the callback of the import button to do what need to be done, or you can also add a new button to import (to avoid conflict with this issue), and then we can refactor the UI at a later stage?

@louisgv

This comment has been minimized.

Copy link
Contributor

louisgv commented Nov 18, 2017

Hmm, seems like #204 and #202 (this) will be sharing some higher order state. I will do some experiment with this UI element as well as implementing a parent state (to keep track of the step for #204) then :-?

@Serhiy-Shekhovtsov

This comment has been minimized.

Copy link
Contributor

Serhiy-Shekhovtsov commented Nov 18, 2017

I am going to do the following:

  • Show if there is any case started. If started - show a message that starting a new case will drop the current one.
  • Show list of images in local system. User can click on image name to view it. (this is already implemented)
  • On "start case" button click - I will drop the current case and create a new one with selected image.
@Serhiy-Shekhovtsov

This comment has been minimized.

Copy link
Contributor

Serhiy-Shekhovtsov commented Nov 18, 2017

@louisgv , but I already started #145 and this one can be closed as a minor part of it.

@Serhiy-Shekhovtsov

This comment has been minimized.

Copy link
Contributor

Serhiy-Shekhovtsov commented Nov 18, 2017

@louisgv I think you can work on #204 separately, right?

@louisgv

This comment has been minimized.

Copy link
Contributor

louisgv commented Nov 19, 2017

@Serhiy-Shekhovtsov For sure! I thought you were looking to avoid conflicting with this issue, but if you can solve both that'd be great :p I didn't mean to raise conflict/confusion.

Also I was spending the entire day updating my build... refer to #234 💀

@Serhiy-Shekhovtsov

This comment has been minimized.

Copy link
Contributor

Serhiy-Shekhovtsov commented Nov 19, 2017

"Import" button UX

Serhiy-Shekhovtsov added a commit to Serhiy-Shekhovtsov/concept-to-clinic that referenced this issue Nov 19, 2017

@Serhiy-Shekhovtsov Serhiy-Shekhovtsov referenced this issue Nov 19, 2017

Closed

Implemented Imagery selection screen #236

1 of 1 task complete

Serhiy-Shekhovtsov added a commit to Serhiy-Shekhovtsov/concept-to-clinic that referenced this issue Nov 19, 2017

Serhiy-Shekhovtsov added a commit to Serhiy-Shekhovtsov/concept-to-clinic that referenced this issue Nov 20, 2017

@isms

This comment has been minimized.

Copy link
Contributor

isms commented Jan 3, 2018

Closed as of #265

@isms isms closed this Jan 3, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment