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 placeholders for text based interactions #4446

Closed
JishnuGoyal opened this issue Jul 22, 2022 · 5 comments · Fixed by #4506
Closed

Add placeholders for text based interactions #4446

JishnuGoyal opened this issue Jul 22, 2022 · 5 comments · Fixed by #4506
Assignees
Labels
Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). Z-ibt Temporary label for Ben to keep track of issues he's triaged.

Comments

@JishnuGoyal
Copy link
Contributor

Is your feature request related to a problem? Please describe.
In the app interactions where the user is expected to type an input as an answer, some users do not understand that they are supposed to type inside the white text box.

Describe the solution you'd like
Add appropriate placeholder text or hints. These placeholders should be similar to the ones shown on the oppia website.

@JishnuGoyal JishnuGoyal self-assigned this Jul 27, 2022
@Broppia Broppia added issue_type_infrastructure Impact: Low Low perceived user impact (e.g. edge cases). labels Jul 29, 2022
@seanlip seanlip added Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). and removed Impact: Low Low perceived user impact (e.g. edge cases). labels Aug 4, 2022
@seanlip seanlip added this to the Alpha MR5 milestone Aug 4, 2022
@seanlip
Copy link
Member

seanlip commented Aug 4, 2022

Noting that this needs to be done for Alpha MR5, for at least TextInput and NumericExpressionInput. It is vital for making the lessons understandable and playable.

@BenHenning
Copy link
Member

I've investigated this further and verified that:

  • All text-based interactions except text input correctly fall back to a built-in placeholder even if the creator overrides the interaction's placeholder with an empty string
  • All text-based interactions have placeholder support except numeric input which uses an always-present hint text
  • The importer hasn't changed since April, and the latest export data seems correct for the placeholder definition
  • The state in question (within Parts of Multiplication Expressions) which was missing a text placeholder does seem to correctly have that placeholder text when viewed locally
  • The code that computes the hint text for text input was introduced as part of the internationalization project 11+ months ago and hasn't changed
  • The hint text disappears as part of Android's TextView if the keyboard is opened and then collapsed, and stays gone unless the user navigates to an earlier card and back

I suspect that the last point is resembling the actual underlying problem here, and it's not obvious how to solve this reliably (since it probably requires altering focus based on whether the keyboard is up, and this is a well-known problem area in Android that can be challenging & unreliable to work with).

Per a discussion with Sean, we're deprioritizing this for the immediate Alpha MR5 release as well as Beta MR1. More product consideration is needed here, and we should consider potential UX approaches that might help mitigate the user confusion without needing to approach this unreliably on the technical side.

@BenHenning
Copy link
Member

Also to clarify one thing: I am going to go ahead and add a default text input placeholder to bring it into conformance with the other text input interactions (just so that the user never sees an empty box except in the keyboard scenario). This is a departure from web behavior, but it's a simple change that seems reasonable.

@BenHenning
Copy link
Member

Correcting one thing from above: per chatting with Sean, we do want to try and get the multi-line hint working since that at least helps mitigate a big part of the problem (as the hint text includes vital information per Sean's earlier comment).

@JishnuGoyal
Copy link
Contributor Author

Thanks for confirming this Ben! I have added the latest code to my PR to handle folding of lines the way we want for our current implementation. Please let me know what are your thoughts! @BenHenning

@BenHenning BenHenning self-assigned this Aug 17, 2022
@BenHenning BenHenning added the Z-ibt Temporary label for Ben to keep track of issues he's triaged. label Sep 17, 2022
BenHenning added a commit that referenced this issue Nov 22, 2022
…nctionality (#4699)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->
fixes #4452
fixes #4453
fixes #4454
fixes #4455
fixes #4456
fixes #4457
fixes #4458 
fixes #4459

This PR introduces the SpotlightFragment into the codebase, which is
essentially a powerful, robust API that can highlight or 'spotlight'
certain parts of the UI to create an onboarding experience for the same.
We introduce the [Spotlight
library](https://github.com/TakuSemba/Spotlight) into the codebase
handles the highlighting of certain elements on screen. The work done in
this PR augments the functionality of the library, by dynamically adding
arrows and hints which create an over-all Spotlight onboarding
experience to the app, beautifully surfacing some functionalities of the
app to a new user.

We also work on making sure that the spotlight is only shown to a user
only once - the protocol buffers are used to save which spotlight has
already been seen.

The purpose of this PR is to introduce an API that the future
contributors can use to seamlessly integrate spotlights onto UI elements
as and when needed, with minimal coding.

This PR also implements spotlights for the onboarding, home, topic and
exploration screens that are required at this time.

For accessibility, the content descriptions of all the elements on which
spotlights are required today are updated. If talkback is turned on, the
spotlights will not show up.

The entire spotlight functionality is also gated behind a feature flag.
This entire PR is backed by tests.

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [x] The PR title and explanation each start with "Fix #bugnum: " (If
this PR fixes part of an issue, prefix the title with "Fix part of
#bugnum: ...".)
- [x] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

## For UI-specific PRs only
<!-- Delete these section if this PR does not include UI-related
changes. -->
If your PR includes UI-related changes, then:
- Add screenshots for portrait/landscape for both a tablet & phone of
the before & after UI changes
- For the screenshots above, include both English and pseudo-localized
(RTL) screenshots (see [RTL
guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility
guide](https://github.com/oppia/oppia-android/wiki/Accessibility-(A11y)-Guide))
- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing

Co-authored-by: Ben Henning <[email protected]>
Co-authored-by: madhurgera2 <[email protected]>
Co-authored-by: JishnuGoyal <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). Z-ibt Temporary label for Ben to keep track of issues he's triaged.
4 participants