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

Landscape support for ItemSelectionInteraction [Blocked on #1737] #1595

Closed
rt4914 opened this issue Aug 10, 2020 · 37 comments
Closed

Landscape support for ItemSelectionInteraction [Blocked on #1737] #1595

rt4914 opened this issue Aug 10, 2020 · 37 comments
Labels
bug End user-perceivable behaviors which are not desirable. Impact: Low Low perceived user impact (e.g. edge cases). Issue: Needs Clarification Indicates that an issue needs more detail in order to be able to be acted upon. Priority: Essential This work item must be completed for its milestone. Z-ibt Temporary label for Ben to keep track of issues he's triaged.

Comments

@rt4914
Copy link
Contributor

rt4914 commented Aug 10, 2020

Describe the bug
If learner has selected few options in ItemSelectionInteraction and does configuration change the selection disappears.

To Reproduce
Steps to reproduce the behavior:

  1. Go to Prototype Exploration

  2. Go to below mentioned state:
    Screenshot_1597062466

  3. Select 2-3 options

  4. Change device configuration

  5. Notice that the selection resets.

Expected behavior
ItemSelectionInteraction should persist selected items on configuration change. Fix this for question and exploration player.

@rt4914 rt4914 added Type: Bug Priority: Essential This work item must be completed for its milestone. labels Aug 10, 2020
@rt4914 rt4914 added this to the Beta milestone Aug 10, 2020
@prayutsu
Copy link
Contributor

@rt4914 I want to work on this issue.

@rt4914
Copy link
Contributor Author

rt4914 commented Aug 24, 2020

@prayutsu This can be a tricky task but considering that you have finished almost 3 PRs, I am assigning this to you. If you get stuck feel free to mention and we will deassign you.

@prayutsu
Copy link
Contributor

@prayutsu This can be a tricky task but considering that you have finished almost 3 PRs, I am assigning this to you. If you get stuck feel free to mention and we will deassign you.

Thank you so much for assigning me the issue and yes, I will surely need your help @rt4914

@prayutsu
Copy link
Contributor

@rt4914 If we could wrap the options selected in a LiveData object. Then the checkboxes will survive configuration change. I explored the directories and packages. But I need a little help in identifying the correct files (like SelectionInteractionViewModel, SelectionInteractionContentViewModel, QuestionPlayerViewModel, etc) where I should try making changes. It would be a pleasure if you could do some help

@aggarwalpulkit596
Copy link
Contributor

@prayutsu things won't work with live data because they cannot retain values on configuration change.

@aggarwalpulkit596
Copy link
Contributor

And you have to make changed ItemSelectionInteractionView

@prayutsu
Copy link
Contributor

@prayutsu things won't work with live data because they cannot retain values on configuration change.

And you have to make changed ItemSelectionInteractionView

Thank you @aggarwalpulkit596 for your help.

@prayutsu
Copy link
Contributor

@aggarwalpulkit596
I searched the file "ItemSelectionInteractionView" and found only this most closely matched file -

src/main/java/org/oppia/app/player/state/itemviewmodel/SelectionInteractionViewModel.kt

Is this what you are referring to?

@aggarwalpulkit596
Copy link
Contributor

Yes that's correct

@prayutsu
Copy link
Contributor

Yes that's correct

Thank you for that @aggarwalpulkit596
I want an extra help.
I am trying to install ktlint and bufbuild on my ubuntu system and for ktlint my terminal says -

**error: The publisher of snap "ktlint" has indicated that they do not consider
       this revision to be of production quality and that it is only meant for
       development or testing at this point. As a consequence this snap will
       not refresh automatically and may perform arbitrary system changes
       outside of the security sandbox snaps are generally confined to, which
       may put your system at risk.
       If you understand and want to proceed repeat the command including
       --devmode; if instead you want to install the snap forcing it into
       strict confinement repeat the command including --jailmode.**

I also tried another command which is there on their official website -
curl -sSLO https://github.com/pinterest/ktlint/releases/download/0.38.1/ktlint && chmod a+x ktlint

But nothing happens, there's not any logs in the terminal and the command isn't executed completely.
Screenshot from 2020-08-26 23-03-32

@BenHenning
Copy link
Member

@vinitamurthi or @anandwana001 is this something that you can help with?

@anandwana001
Copy link
Contributor

Yes that's correct

Thank you for that @aggarwalpulkit596
I want an extra help.
I am trying to install ktlint and bufbuild on my ubuntu system and for ktlint my terminal says -

**error: The publisher of snap "ktlint" has indicated that they do not consider
       this revision to be of production quality and that it is only meant for
       development or testing at this point. As a consequence this snap will
       not refresh automatically and may perform arbitrary system changes
       outside of the security sandbox snaps are generally confined to, which
       may put your system at risk.
       If you understand and want to proceed repeat the command including
       --devmode; if instead you want to install the snap forcing it into
       strict confinement repeat the command including --jailmode.**

I also tried another command which is there on their official website -
curl -sSLO https://github.com/pinterest/ktlint/releases/download/0.38.1/ktlint && chmod a+x ktlint

But nothing happens, there's not any logs in the terminal and the command isn't executed completely.
Screenshot from 2020-08-26 23-03-32

As per the wiki mentioned, we are currently using 0.37.0 version of the ktlint. Please check with this version and let me know if the same issue occurs.

@prayutsu
Copy link
Contributor

@aggarwalpulkit596 I am trying to solve this issue by using savedStateHandle in SelectionInterActionViewModel
We can save the currently selected options in a key-value pair. And after configuration changes, when the ViewModel is created again, in the init block we can check if the state is not null, we use the stored values in selectedItems. This should work, right?

@prayutsu
Copy link
Contributor

@anandwana001 Even after trying version 0.37.0, still the same thing is happening.
Screenshot from 2020-08-27 12-05-35

@anandwana001
Copy link
Contributor

anandwana001 commented Aug 27, 2020

@anandwana001 Even after trying version 0.37.0, still the same thing is happening.
Screenshot from 2020-08-27 12-05-35

Just to confirm before I look into what's the issue, apart from ktlint does curl commands working on your machine?
Also, have you tried with brew - brew install ktlint

@prayutsu
Copy link
Contributor

@anandwana001 Even after trying version 0.37.0, still the same thing is happening.
Screenshot from 2020-08-27 12-05-35

Just to confirm before I look into what's the issue, apart from ktlint does curl commands working on your machine?
Also, have you tried with brew - brew install ktlint

yes, they work

@anandwana001
Copy link
Contributor

@anandwana001 Even after trying version 0.37.0, still the same thing is happening.
Screenshot from 2020-08-27 12-05-35

Just to confirm before I look into what's the issue, apart from ktlint does curl commands working on your machine?
Also, have you tried with brew - brew install ktlint

yes, they work

ok let me see, The common issue always is this command sets up the file which requires stable and strong internet connection.

@prayutsu
Copy link
Contributor

@anandwana001 Please take a look on this
Screenshot from 2020-08-27 13-03-37

@anandwana001
Copy link
Contributor

Could you try this one - https://snapcraft.io/install/ktlint/ubuntu

@prayutsu
Copy link
Contributor

Could you try this one - https://snapcraft.io/install/ktlint/ubuntu

Screenshot from 2020-08-27 13-10-28

@rt4914
Copy link
Contributor Author

rt4914 commented Aug 27, 2020

@anandwana001 @aggarwalpulkit596 @prayutsu

Please avoid having conversations on issues, instead you can chat in gitter channel or on email.
Thanks

@rt4914 rt4914 changed the title Landscape support for ItemSelectionInteraction Landscape support for ItemSelectionInteraction [Blocked on #1737] Aug 28, 2020
@prayutsu prayutsu removed their assignment Aug 28, 2020
@prayutsu
Copy link
Contributor

@rt4914 I am removing my assignment for this issue, so can you please assign me new issues?

@Broppia Broppia added issue_type_bug Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). labels Jul 29, 2022
@BenHenning BenHenning added Impact: Low Low perceived user impact (e.g. edge cases). Issue: Needs Clarification Indicates that an issue needs more detail in order to be able to be acted upon. Z-ibt Temporary label for Ben to keep track of issues he's triaged. issue_user_learner and removed Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). labels Sep 15, 2022
@BenHenning BenHenning removed this from the Beta milestone Sep 16, 2022
@seanlip seanlip added bug End user-perceivable behaviors which are not desirable. and removed issue_user_learner labels Mar 29, 2023
@rt4914
Copy link
Contributor Author

rt4914 commented Apr 12, 2023

Duplicate #4472

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug End user-perceivable behaviors which are not desirable. Impact: Low Low perceived user impact (e.g. edge cases). Issue: Needs Clarification Indicates that an issue needs more detail in order to be able to be acted upon. Priority: Essential This work item must be completed for its milestone. Z-ibt Temporary label for Ben to keep track of issues he's triaged.
Development

Successfully merging a pull request may close this issue.

7 participants