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

Fix #151: Lowfi numeric input interaction view part 2 #223

Conversation

nikitamarysolomanpvt
Copy link
Contributor

@nikitamarysolomanpvt nikitamarysolomanpvt commented Oct 10, 2019

Replicated from #190

Explanation

Part 1: #220
Part 2: This PR contains Numeric input view and Fix #151 .

Reference Mock

https://xd.adobe.com/spec/e2239cf4-9cde-4c08-5296-25316c1f0a14-9412/screen/c4b0dfb3-53b4-491e-a759-818e096183fd/Lesson-1-a-v-Exploration-Player-Numeric-Input-No-A

Reference Design Doc

https://docs.google.com/document/d/1QigNdZeMBy-JjHW26QH4qT0UtCyzfvW0xEaStSdY9gM/edit?usp=sharing

Checklist

  • The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
  • The PR explanation includes the words "Fixes #bugnum: ..." (or "Fixes part of #bugnum" if the PR only partially fixes an issue).
  • The PR follows the style guide.
  • The PR does not contain any unnecessary auto-generated code from Android Studio.
  • The PR is made from a branch that's not called "develop".
  • The PR is assigned to an appropriate reviewer.

…d NumericInput from dummy data welcome.json ,welcome.json file "init_state_name"modified to "Numeric input",
… numbertextinputlayout onbutton click. on button click listner implemented
@rt4914
Copy link
Contributor

rt4914 commented Oct 10, 2019

@nikitamarysolomanpvt Approach is correct. You can proceed ahead and finish this PR.

@nikitamarysolomanpvt
Copy link
Contributor Author

@nikitamarysolomanpvt Approach is correct. You can proceed ahead and finish this PR.

Ok Thank You

@nikitamarysolomanpvt nikitamarysolomanpvt changed the title Fix part of #151: Text input lowfi numeric input interaction view part 2(DO NOT MERGE) Fix part of #151: Text input lowfi numeric input interaction view part 2 Oct 10, 2019
@rt4914
Copy link
Contributor

rt4914 commented Oct 11, 2019

@nikitamarysolomanpvt

Regarding PR description.

  1. Do not mention that test-case is in progress in description, better mention this in a top-level comment.
  2. You have linked this PR to Fix #150, #151, #155: TextInputInteractionView, NumericInputInteractionView, FractionInputInteractionView #190 , I would suggest linking this current PR to Fix #150, #151, #155: TextInputInteractionView, NumericInputInteractionView, FractionInputInteractionView #190 and close Fix #150, #151, #155: TextInputInteractionView, NumericInputInteractionView, FractionInputInteractionView #190
  3. Reference link to mock is incorrect, please update that.

Copy link
Contributor

@rt4914 rt4914 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have some nit changes to do.

@nikitamarysolomanpvt
Copy link
Contributor Author

@nikitamarysolomanpvt

Regarding PR description.

  1. Do not mention that test-case is in progress in description, better mention this in a top-level comment.
  2. You have linked this PR to Fix #150, #151, #155: TextInputInteractionView, NumericInputInteractionView, FractionInputInteractionView #190 , I would suggest linking this current PR to Fix #150, #151, #155: TextInputInteractionView, NumericInputInteractionView, FractionInputInteractionView #190 and close Fix #150, #151, #155: TextInputInteractionView, NumericInputInteractionView, FractionInputInteractionView #190
  3. Reference link to mock is incorrect, please update that.

Okay

@nikitamarysolomanpvt
Copy link
Contributor Author

Test case is in progress

@@ -160,7 +160,14 @@
}
],
"confirmed_unclassified_answers": [],
"customization_args": {},
"customization_args": {
Copy link
Contributor Author

@nikitamarysolomanpvt nikitamarysolomanpvt Oct 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is added for testing numeric input interaction view

Copy link
Member

@BenHenning BenHenning left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @nikitamarysolomanpvt. Took a deep dive on the PR. Please let me know if you have any questions.

attributes()
}

constructor(context: Context?, placeholder: String) : this(context!!) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to dynamically create the view? I wouldn't expect that since we're inflating it using LayoutInflater rather than directly constructing it.

launchedActivity = activityTestRule.launchActivity(intent)
onView(withId(R.id.test_number_input_interaction_view)).perform(ViewActions.clearText(), ViewActions.typeText("9"))
activityTestRule.activity.requestedOrientation = ActivityInfo.SCREEN_ORIENTATION_LANDSCAPE
activityTestRule.activity.requestedOrientation = ActivityInfo.SCREEN_ORIENTATION_PORTRAIT
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't sufficient to trigger a configuration change. We can use ActivityScenario.recreate() for this purpose. The orientation change actually may not be needed; recreate may be enough to fully recreate the activity.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nikitamarysolomanpvt Check TopicTrainFragmentTest file you can do this similarly here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its pending

Copy link
Member

@BenHenning BenHenning left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @nikitamarysolomanpvt. This LGTM! Please feel free to submit once my comments are addressed.

@nikitamarysolomanpvt nikitamarysolomanpvt merged commit 52f11b6 into develop Oct 21, 2019
@rt4914 rt4914 mentioned this pull request Oct 22, 2019
6 tasks
nikitamarysolomanpvt added a commit that referenced this pull request Oct 23, 2019
…View low-fi (#246)

* NumberInputInteractionView dynamically created on state interaction id NumericInput from dummy data welcome.json ,welcome.json file "init_state_name"modified to "Numeric input",

* NumberInputInteractionView dynamically created on state interaction id NumericInput from dummy data welcome.json

* added fetch button, and fetch textview to fetch the data from dynamic numbertextinputlayout onbutton click. on button click listner implemented

* nit changes

* nit changes

* nit changes

* nit changes

* nit changes

* numericInputTypeView test case is in progress

* numericInputTypeView test case is in progress

* nit changes as per #223

* nit changes as per #223

* On configuration changes save and restore the text of numericInputType

* Test Cases for NumericInputInteractionView,
Moved test case to StateFragment

* Test Cases for NumericInputInteractionView,
Moved test case to StateFragment,PR suggestions updated

* PR suggestions updated

* PR suggestions updated

* Merge branch 'develop' of https://github.com/oppia/oppia-android into topic-player-multiple-tabs

# Conflicts:
#	app/build.gradle
#	app/src/main/AndroidManifest.xml
#	app/src/main/res/values/styles.xml

* nit changes as per PR review

* nit changes as per PR review

* nit changes as per PR review

* nit changes as per PR review

* Number input test Activity test cases

* nit changes

* nit changes

* nit changes

* nit changes

* nit changes

* nit changes

* nit changes

* nit changes

* nit changes

* nit changes

* nit changes

* setReal type in getPendingAnswer

* updated test cases, disabled long click to prevent pasting value

* nit changes

* nit changes

* nit changes

* nit changes

* nit changes

* nit changes

* nit changes

* nit changes

* nit changes

* NumericInputInteractionViewTestActivity renamed to InputInteractionViewTestActivity and test case updated

* nit changes

* nit changes

* nit changes

* nit

* NumericInputInteractionView attributes are moved to xml,testcases updated as per PR review of Ben, other nit changes

* NumericInputInteractionView attributes are moved to xml,testcases updated as per PR review of Ben, other nit changes

* TextInputInteractionView

* TextInputInteractionView

* test cases

* nit

* nit

* nit

* nit

* nit

* Merge branches 'develop' and 'lowfi-input-interaction-views' of https://github.com/oppia/oppia-android into lowfi-input-interaction-views

# Conflicts:
#	app/src/main/java/org/oppia/app/activity/InputInteractionViewTestActivity.kt
#	app/src/main/java/org/oppia/app/customview/interaction/NumericInputInteractionView.kt
#	app/src/main/res/layout/activity_numeric_input_interaction_view_test.xml
#	app/src/main/res/values/dimens.xml
#	app/src/sharedTest/java/org/oppia/app/activity/InputInteractionViewTestActivityTest.kt

* nit

* nit

* nit

* nit

* Merge branches 'develop' and 'text-input-lowfi-numeric-input-interaction-view-part-2' of https://github.com/oppia/oppia-android into text-input-lowfi-numeric-input-interaction-view-part-2

# Conflicts:
#	app/src/sharedTest/java/org/oppia/app/activity/InputInteractionViewTestActivityTest.kt

* nit

* NumberWithUnitsInputInteractionView

* nit

* nit

* NumberWithUnitsInputInteractionView

* moved InputInteractionViewTestActivityTest to testing folder
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UI Structure: NumericInputInteractionView [Blocked: #150]
4 participants