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

Improve styling of mRDT Enketo widget #4745

Closed
abbyad opened this issue Jul 19, 2018 · 8 comments
Closed

Improve styling of mRDT Enketo widget #4745

abbyad opened this issue Jul 19, 2018 · 8 comments
Labels
Priority: 3 - Low Can be bumped from the release Type: Improvement Make something better

Comments

@abbyad
Copy link
Contributor

abbyad commented Jul 19, 2018

The following changes are requested for the Enketo widget for mRDT:

  1. Add a space between the image and the take photo button. @amandacilek can specify the exact amount.
  2. Hide the text input. Prior to getting an image it should only show the take photo button. Afterwards it will have the image and the take photo button in case they want to replace the one that was taken.

This is what it currently looks like:

image

@abbyad abbyad added Status: 1 - Triaged Priority: 3 - Low Can be bumped from the release Type: Improvement Make something better labels Jul 19, 2018
@garethbowen garethbowen self-assigned this Jul 23, 2018
@amandacilek-zz
Copy link

amandacilek-zz commented Jul 23, 2018

I'd give it a gap of at least 30px (between the bottom of the photo and the "take photo" button.) If the grey bar with random text above the image is going away then we should have room.

Question - in the screenshot above the photo has already been taken. If this is true, could we change the button text after the fact? To something like "Re-take photo" or "Replace photo"? It seems confusing to have already taken the photo and still see a button that says "Take Photo".

Another nitpick - usually text is configurable but if it isn't for this widget, can we please capitalize the button text so the first letter of each word is capitalized? Ex: "Take Photo" instead of "take photo".

@garethbowen
Copy link
Member

@amandacilek As part of making this production ready all the text will be internationalised so it can be configured and I'll definitely make the default capitalised!

I think the button should be above the preview rather than below it. This makes more sense if you're working from the top of the form to the bottom. Does this work around your suggestion of changing the button text?

@amandacilek-zz
Copy link

I think moving the button so it is above the photo is a good alternative if we don't want to mess with changing the text inside the button. It makes the order of operations clearer at least - you've clicked the button, you've added a photo, and then the next thing you see is the Submit button. I'm fine with that solution, thanks.

@garethbowen
Copy link
Member

I'd give it a gap of at least 30px (between the bottom of the photo and the "take photo" button.)

30px looked strange because the paddings between the other elements are 8px top and bottom (16px total). I've stuck with this to keep consistency. We should investigate adding more whitespace in a wider Enketo styling review.

can we please capitalize the button text so the first letter of each word is capitalized? Ex: "Take Photo" instead of "take photo".

We use sentence case (not title case) elsewhere so I've made the default translation "Take photo" (capital t, lowercase p).

This is how it looks now, before taking the photo:

before

... and after taking the photo:

after

@amandacilek-zz
Copy link

I'm fine with all of this and your two compromises.

Separately though I have another thought as I look at these screenshots - Are we requiring the user to take a photo or is it optional? In the first screenshot, the "Submit" button stands out more than the "Take photo" button. So a user might be inclined to just click the submit button and pass over the take photo button altogether.

Is it possible to make the initial take photo button blue so it stands out more, just like the Submit button?

I like the after the fact take photo button as is (more of a ghost button)

@garethbowen
Copy link
Member

I don't think making it the same weight as the submit/next button is the ideal solution. Perhaps we can work on the ideal style in a later release?

FYI if the field is marked as required but the user misses the button and hits next, it looks like this:

required

@amandacilek-zz
Copy link

I'm not sure I agree with you about the button styling. I think a button should be a clear and obvious button (blue) whenever possible, and only NOT blue in specific circumstances (for example we want to de-emphasize the "Cancel" button because of how dangerous it is).

I'm willing to hold my peace for now though. This is the first I've seen of what the integration looks like in the app so I'm just voicing the suggestions that come to mind. I don't know if we're making the image required or not but perhaps this is a good rationale of why we would want to do so.

garethbowen added a commit that referenced this issue Jul 26, 2018
This adds a new widget for MRDT verification which launches a third party app which
returns a photo of the MRDT result which gets attached to the report and displayed in
the report view.

Attaching binary content to the report is done generically so other widgets can use the
same pattern.

#4743
#4745
#4742
garethbowen added a commit that referenced this issue Jul 26, 2018
This adds a new widget for MRDT verification which launches a third party app which
returns a photo of the MRDT result which gets attached to the report and displayed in
the report view.

Attaching binary content to the report is done generically so other widgets can use the
same pattern.

#4743
#4745
#4742
garethbowen added a commit that referenced this issue Jul 26, 2018
This adds a new widget for MRDT verification which launches a third party app which
returns a photo of the MRDT result which gets attached to the report and displayed in
the report view.

Attaching binary content to the report is done generically so other widgets can use the
same pattern.

#4743
#4745
#4742
@garethbowen garethbowen removed their assignment Jul 26, 2018
@newtewt
Copy link
Contributor

newtewt commented Aug 1, 2018

@amandacilek @garethbowen @abbyad When tapping the "Take photo" button we launch the MRDT app with the image scan open. Pressing the hardware back button returns to the MRDT app and not our app. Pressing back again returns to our app. I feel like this isn't the best user experience but would defer to the experts. Not sure if we can register the back in their app to return to ours or ask for a cancel button or x to be added so we are returned directly to our app vs the MRDT app. Again, I don't think this is something we should hold up for but maybe a next iteration thought.

Otherwise this looks good and I'm closing the ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: 3 - Low Can be bumped from the release Type: Improvement Make something better
Projects
None yet
Development

No branches or pull requests

4 participants