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.

Sign up for GitHub

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

Jump to bottom

ItemSelectionInteraction text change on selection #3169

Closed
rt4914 opened this issue May 6, 2021 · 27 comments · Fixed by #4777
Closed

ItemSelectionInteraction text change on selection #3169

rt4914 opened this issue May 6, 2021 · 27 comments · Fixed by #4777
Assignees
@Akshatkamboj14
Labels
good first issue This item is good for new contributors to make their pull request. Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). Priority: Essential This work item must be completed for its milestone. Type: Task A single task of work corresponding to a greater milestone. Generally corresponds to a single PR. Z-ibt Temporary label for Ben to keep track of issues he's triaged.

Comments

@rt4914
Copy link
Contributor

rt4914 commented May 6, 2021 •
edited by seanlip
Loading

EDIT (by @seanlip): See updated description here -- #3169 (comment)

Previous description

Mock1

As per Mock1, by default the text is Please select atleast one choice.. If atleast one option is selected we hide this text and if the option is deselected then we show this text again.

Add this functionality and its relevant test.

Expected Cases:

  1. No item selected - Default Text - Please select one or more choices.
  2. If total selection count is atleast 1 and less than allowable selections, change text to You may select more choices.
  3. If total selection count is equal to allowable selections, change text to No more than 4 choices may be selected.

You can experiment similar cases on What is a Fraction? . You will need to answer a lot of states before you will reach a state where you will find this interaction where you can select multiple options.
@rt4914 rt4914 added good first issue This item is good for new contributors to make their pull request. Type: Task A single task of work corresponding to a greater milestone. Generally corresponds to a single PR. Where: Exploration player Priority: Essential This work item must be completed for its milestone. labels May 6, 2021
@rt4914 rt4914 added this to the Beta milestone May 6, 2021
@rt4914
Copy link
Contributor Author

rt4914 commented May 6, 2021

@mschanteltc Can you answer the question mentioned in the issue?

@mschanteltc
Copy link

I'm thinking we should remove the "You may select more choices" altogether because it sounds awkward, and it would be contradicting to leave it there if all options were selected.

Let's keep "Please select at least one choice" there and remove it when the user selects their first choice. If the user deselects the one selected option, the label appears again. WDYT?

@rt4914
Copy link
Contributor Author

rt4914 commented May 12, 2021

I'm thinking we should remove the "You may select more choices" altogether because it sounds awkward, and it would be contradicting to leave it there if all options were selected.

Let's keep "Please select at least one choice" there and remove it when the user selects their first choice. If the user deselects the one selected option, the label appears again. WDYT?

@mschanteltc Sounds good. Will apply this. Thanks.

@rt4914 rt4914 mentioned this issue May 20, 2021
Closed
@anandwana001
Copy link
Contributor

@priyanka0906 any update on this?

@priyanka0906
Copy link
Contributor

@priyanka0906 any update on this?

@anandwana001 not yet.

@anandwana001
Copy link
Contributor

anandwana001 commented Jun 22, 2021 •
edited
Loading

@priyanka0906 Let me know if you are still working on this, or we can unassign and look for other interesting issues.

@priyanka0906
Copy link
Contributor

@priyanka0906 Let me know if you are still working on this, or we can unassign and look for other interesting issues.

@anandwana001 I am currently not working on this.

@sanchi0204
Copy link

Can I work on this?

@rt4914
Copy link
Contributor Author

rt4914 commented Aug 4, 2021

@sanchi0204 Any updates?

@sanchi0204
Copy link

@sanchi0204 Any updates?
Hey @rt4914, sorry for the delay, but my university exams have been preponed. Will make a PR probably within a week.

@rt4914
Copy link
Contributor Author

rt4914 commented Aug 4, 2021

Okay, in that case I will un-assign you for now and feel free to take any new issue when you are available again. Thanks and all the best.

@Rohit1173
Copy link
Contributor

May I work on this

@Rohit1173
Copy link
Contributor

@anandwana001 I am not able to find this screen on my mobile app, Could you pls guide me through

@anandwana001
Copy link
Contributor

Steps:

  1. Open Fractions card on the home screen
  2. Lesson Tab
  3. Expand the card
  4. Start playing chapter 1 "What is Fraction?"
  5. While playing this fraction chapter you will get questions where they have multiple options to select.

This issue is not just about the 1 question but it's about all the questions which have multiple answers to select.

Let me know if this helps you or not.

@Rohit1173
Copy link
Contributor

Yeah, Thanks @anandwana001

@Broppia Broppia added issue_type_bug Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). and removed issue_type_bug labels Jul 29, 2022
@Akshatkamboj14 Akshatkamboj14 mentioned this issue Jul 31, 2022
Closed
6 tasks
@seanlip
Copy link
Member

seanlip commented Aug 24, 2022 •
edited
Loading

We changed how we display this in #4491 per learner/partner feedback. So, let's leave the text as-is until we get more feedback that the new version isn't suitable. /cc @BenHenning

However I have a question. When the user selects the max number of choices, can we disable the rest? I think that's probably the only thing we need to do here.

@seanlip
Copy link
Member

seanlip commented Aug 24, 2022

Also, @Akshatkamboj14, with this clarification, would you be comfortable working on this?

@BenHenning
Copy link
Member

We changed how we display this in #4491 per learner/partner feedback. So, let's leave the text as-is until we get more feedback that the new version isn't suitable. /cc @BenHenning

However I have a question. When the user selects the max number of choices, can we disable the rest? I think that's probably the only thing we need to do here.

Disabling checkboxes should indeed be possible, though we'll need to test how Talkback behaves for those checkboxes to make sure the experience is equally seamless (or we may need to refine the instructional language to accommodate both screenreader & non-screenreader users).

@Akshatkamboj14
Copy link
Member

@seanlip yes, I am comfortable working on this.

@seanlip
Copy link
Member

seanlip commented Aug 30, 2022

OK, thanks! Feel free to ask if any questions come up.

@Akshatkamboj14
Copy link
Member

Hey, @seanlip I want to confirm that we need to change the default text to "Please select all correct choices" instead of "Please select one or more choices." and we need to disable the other options after selecting the max number of choices, is that all? or there are other things as well?

@seanlip
Copy link
Member

seanlip commented Aug 31, 2022

This is correct, but the first one is done already right? See #4493.

@Akshatkamboj14
Copy link
Member

Okay, @seanlip is there anything else except this?

@seanlip
Copy link
Member

seanlip commented Sep 1, 2022

No, that's all (assuming that by "this" you mean the second thing you mentioned, i.e. disabling the other options).

@Akshatkamboj14
Copy link
Member

Okay thanks @seanlip

@Akshatkamboj14 Akshatkamboj14 mentioned this issue Sep 11, 2022
Closed
6 tasks
@BenHenning BenHenning added Z-ibt Temporary label for Ben to keep track of issues he's triaged. issue_user_learner labels Sep 15, 2022
@BenHenning BenHenning removed this from the Beta milestone Sep 16, 2022
@BenHenning BenHenning moved this from Needs Triage to In Progress: Being implemented in [Team] Core Learner and Mastery flows & UI Frontend - Android Sep 20, 2022
@Akshatkamboj14 Akshatkamboj14 mentioned this issue Oct 2, 2022
Closed
6 tasks
@Akshatkamboj14 Akshatkamboj14 mentioned this issue Nov 30, 2022
Merged
6 tasks
rt4914 pushed a commit that referenced this issue Mar 2, 2023
@Akshatkamboj14 @BenHenning
Fixes #3169: Item selection interaction text change and checkboxes sh…
c1ce5f8 
…ould be disabled on selection (#4777)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
Fixes #3169
Fixes #3624

This PR is doing 2 things - 

1. It's updating the textView based on the selection of checkboxes by
the user, If the user has not selected any checkbox then the Text will
be `Please select all correct choices.`, If the user has selected less
than maxSelection then the text will be `You may select more choices.`
and if the user has selected maxSelection then the text will be `No more
than $maxSelection choices may be selected.`

2. If the user has selected maximum checkboxes that can be selected,
then the non selected checkboxes will be disbaled, and if the user
unselects the checkbox or checkboxes, then all checkboxes will get
enabled.

This is the link to the repository for the comparison of the
StateFragmentTest.kt, to show that the same tests are failing on both
branches(i.e. my feature branch and my develop branch)

https://github.com/Akshatkamboj14/Test-Comparison-For-StateFragmentTest.kt-oppia-android

<!--
- 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.
  -->

## 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


[issue.webm](https://user-images.githubusercontent.com/92685493/221786503-45d2c027-bedd-4cfb-92ce-3e351e81c34b.webm)

---------

Co-authored-by: Ben Henning <ben@oppia.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Akshatkamboj14 Akshatkamboj14

Labels
good first issue This item is good for new contributors to make their pull request. Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). Priority: Essential This work item must be completed for its milestone. Type: Task A single task of work corresponding to a greater milestone. Generally corresponds to a single PR. Z-ibt Temporary label for Ben to keep track of issues he's triaged.
Projects
[Team] Core Learner and Mastery flows & UI Frontend - Android
Archived in project
Milestone
No milestone