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

Revealing second hint after orientation change before the timer #2476

Closed
FareesHussain opened this issue Jan 14, 2021 · 7 comments · Fixed by #3659
Closed

Revealing second hint after orientation change before the timer #2476

FareesHussain opened this issue Jan 14, 2021 · 7 comments · Fixed by #3659
Assignees
@MaskedCarrot @yashraj-01
Labels
Z-ibt Temporary label for Ben to keep track of issues he's triaged.

Comments

@FareesHussain
Copy link
Contributor

Describe the bug
For the questions with more than one hint, the second hint is revealed before 10 seconds after orientation change

To Reproduce
Steps to reproduce the behavior:

  1. Go to What is a Fraction story inside Fractions
  2. click on continue, continue, Three fifths, Four sevenths, 2nd option (green)
  3. Here select the option 7/2 twice and reveal the hint
  4. go back to question
  5. change to the landscape
  6. see the 2nd revealed hint

Expected behavior
Only reveal second hint after 10sec timer
@FareesHussain
Copy link
Contributor Author

cc: @BenHenning

@BenHenning
Copy link
Member

So I think that this might because of this line. It seems like in some cases it might force a new hint to be shown when compute() is called (which will happen on configuration change since the adapter & list is recreated). I'm guessing that we need to build in some additional check to ensure we're only ever showing a hint when there's legitimately a new one to show.

@BenHenning
Copy link
Member

We should also make sure that this specific cases & related cases are thoroughly tested, too.

@FareesHussain
Copy link
Contributor Author

@BenHenning Can we do something like restart the timer after orientation change

@rt4914 rt4914 added this to the Beta milestone Jan 15, 2021
@imraghav20
Copy link
Contributor

@FareesHussain can I work on this issue?

@imraghav20
Copy link
Contributor

I tried to make few changes, although the issue is still not solved.
Should I share the changes in code in a comment or make a PR to review and suggest appropriate changes.

@FareesHussain
Copy link
Contributor Author

I'm sorry I forgot to assign myself actually I'm currently working on it

@FareesHussain FareesHussain self-assigned this Jan 19, 2021
@FareesHussain FareesHussain mentioned this issue Jan 24, 2021
Closed
8 tasks
This was referenced Mar 10, 2021
Closed
Closed
@MaskedCarrot MaskedCarrot mentioned this issue Aug 8, 2021
Merged
8 tasks
BenHenning added a commit that referenced this issue Aug 19, 2021
@MaskedCarrot @yashraj-01 @BenHenning
Fix #2476, #3312 and #3703: Move hint handler to domain layer (#3659)
d2372ae 
* moved hint handler to domain layer

* fixed app layer espresso tests

* fix app layer robolectric tests

* fixed domain layer tests

* Added annotations to test exemptions

* proto lint fix

* fixed hint handler for training sessions

* nit and removed test excemptions

* added hint tests for config change

* fixed test file exemptions

* fixed failing test

* made HintHandler injectable

* fixed ktlint error

* Added tests for hint handler

* nits

* fixed failing build

* fixed failing build

* fix build

* fixed imports

* nits and improved testing

* updated exploration.proto

* removed progress controller from kdoc exemptions

* fix failing test

* moved timer to domain

* fixed build and nits

* nit

* added listener back to test exemptions

* nit fixes and added more tests

* lint fix

* First round: make HintHandler independent.

This also brings HintHandler into an interface + factory pattern. This
isn't the final design since I think we can largely simplify the way
hints work; that'll be my next pass.

This breaks questions & HintHandlerTest; those will require further work
later.

* Simplify HelpIndex in proto.

Move HelpIndex to PendingState to avoid the entire domain case of
handling CompletedState.

* Simplify hints & solutions.

This removes the per-Hint/Solution tracking & completely leans on
HelpIndex for proper hints & solution tracking both in the domain & UI
layers.

Fixes a bunch of other stuff, too, including
QuestionAssessmentProgressController tests.

* Clean up dead code paths & improve handler API.

* fixed test modules and lint

* renamed HintHandlerTest to HintHandlerImplTest

* Add tests for HintHandler.

This introduces some new explorations for making testing HintHandlerImpl
easier.

* Add remaining tests/exemptions for new files.

* Lint fixes.

* Post-merge fixes (including lint fixes).

* Post-merge maven_install fix.

* Revert "Merge branch 'develop' into move-hint-handler-to-domain"

This reverts commit e2fea90, reversing
changes made to 6659858.

* Post-merge Gradle-discovered fixes.

* Revert "Revert "Merge branch 'develop' into move-hint-handler-to-domain""

This reverts commit b1622c0.

* Additional post-merge fixes.

* Address first batch of review comments.

(Clarified some proto fields & remove trailing comma).

* Fix testing module tests & remove unnecessary changes.

* Simplify & fix more reviewer comments.

This simplifies some data pipelining in the UI, and improves HintHandler
documentation in addition to fixing some more reviewer comments.

* Improve documentation to address review comment.

* Rename new module to prod module.

This simplifies the changes & approvals needed in #3705.

* Rename proto field (to address review comment).

Also, fix broken reference error accidentally introduced in an earlier
commit.

* Kotlin lint fixes.

Co-authored-by: yashraj-01 <[email protected]>
Co-authored-by: Ben Henning <[email protected]>
@BenHenning BenHenning added the Z-ibt Temporary label for Ben to keep track of issues he's triaged. label Sep 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@MaskedCarrot MaskedCarrot

@yashraj-01 yashraj-01

Labels
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
6 participants
@rt4914 @BenHenning @MaskedCarrot @FareesHussain @imraghav20 @yashraj-01