Merge multiple versions of audio_language_fragment.xml #3997

jashasweejena · 2021-11-18T18:19:57Z

Explanation

Fixes #3462 :

Unified multiple versions of audio_language_fragment.xml and put the resolution-specific values into their respective dimens.xml files.
Move Toolbar from audio_language_fragment.xml to audio_language_activity.xml

Essential Checklist

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: ...".)
Any changes to scripts/assets files have their rationale included in the PR explanation.
The PR follows the style guide.
The PR does not contain any unnecessary code changes from Android Studio (reference).
The PR is made from a branch that's not called "develop" and is up-to-date with "develop".
The PR is assigned to the appropriate reviewers (reference).

For UI-specific PRs only

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)
Add a video showing the full UX flow with a screen reader enabled (see accessibility guide)
Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing

Screenshots

Mobile Portrait Before	Mobile Portrait After

Mobile Landscape Before	Mobile Landscape After

Tablet Portrait Before	Tablet Portrait After

Tablet Landscape Before	Tablet Landscape After

…d set click listeners in AudioLanguageActivityPresenter

…om xml and Presenter

… and use resolution specific dimens.xml to separate dimensions

rt4914 · 2021-11-24T04:03:05Z

@jashasweejena Apologies for the delay. Have you received any welcome email from a team-member? Have you been assigned a mentor yet?

jashasweejena · 2021-11-24T04:32:23Z

@jashasweejena Apologies for the delay. Have you received any welcome email from a team-member? Have you been assigned a mentor yet?

Hey @rt4914, actually I haven't received any welcome mail yet nor have I been assigned a mentor.

rt4914 · 2021-11-24T04:38:17Z

@jashasweejena Apologies for the delay. Have you received any welcome email from a team-member? Have you been assigned a mentor yet?

Hey @rt4914, actually I haven't received any welcome mail yet nor have I been assigned a mentor.

@jashasweejena Have you followed the steps mentioned here? Mainly survey form and CLA.
Checkout our wiki first. This will help you in setting up the project and once you sign the CLA and fill the survey form we can assign you issues to work on.

jashasweejena · 2021-11-24T04:57:19Z

@jashasweejena Apologies for the delay. Have you received any welcome email from a team-member? Have you been assigned a mentor yet?

Hey @rt4914, actually I haven't received any welcome mail yet nor have I been assigned a mentor.

@jashasweejena Have you followed the steps mentioned here? Mainly survey form and CLA.
Checkout our wiki first. This will help you in setting up the project and once you sign the CLA and fill the survey form we can assign you issues to work on.

Yeah surely. Thanks for mentioning it. I'll make sure to fill those up.

rt4914

@jashasweejena Added few comments.

@bkaur-bkj PTAL and please verify that the functionality is correct.

app/src/main/java/org/oppia/android/app/options/AudioLanguageActivityPresenter.kt

jashasweejena · 2021-11-26T18:34:04Z

Hey @rt4914, thanks for the helpful comments. I will fix them asap.

bkaur-bkj · 2021-11-27T09:14:34Z

@jashasweejena Added few comments.

@bkaur-bkj PTAL and please verify that the functionality is correct.

@rt4914 yes sir it works fine on phone, though not much aware of the databinding changes that needs to be done , so pls you just have a view over it.

jashasweejena · 2021-11-27T10:47:45Z

Hey @rt4914, I completed the mentioned changes and pushed the code.

bkaur-bkj · 2021-12-02T10:09:10Z

Hey @rt4914, I completed the mentioned changes and pushed the code.

@jashasweejena make sure you assign the reviewer and unassign yourself after completing your work

bkaur-bkj · 2021-12-02T10:09:42Z

@jashasweejena you can just write PTAL @Reviewer

bkaur-bkj · 2021-12-02T10:13:37Z

@jashasweejena link checks are failing maybe there is some indentation or spacing issue PTAL over it

bkaur-bkj · 2021-12-02T10:14:09Z

@rt4914 PTAL

jashasweejena · 2021-12-02T11:56:11Z

@jashasweejena you can just write PTAL @Reviewer

Thanks for informing. I'll take care next time onwards.

jashasweejena · 2021-12-02T11:56:28Z

@jashasweejena link checks are failing maybe there is some indentation or spacing issue PTAL over it

Yeah sure, will take a look over it.

oppiabot · 2021-12-02T11:59:14Z

Unassigning @jashasweejena since a re-review was requested. @jashasweejena, please make sure you have addressed all review comments. Thanks!

…ings

jashasweejena · 2021-12-04T20:36:53Z

@rt4914 @bkaur-bkj Fixed the Ktlint issues. PTAL.

Thanks.

rt4914 · 2021-12-07T05:00:43Z

@bkaur-bkj Will review it once you have approved it. Thanks.

bkaur-bkj · 2021-12-07T10:02:29Z

app/src/main/res/layout/audio_language_activity.xml

+      app:layout_constraintTop_toBottomOf="@id/reading_list_app_bar_layout" />
+  </androidx.constraintlayout.widget.ConstraintLayout>
+
+</layout>


@jashasweejena here you need to add a newline, as it says no newline at end of file. Just take cursor to last line and press enter

Fixed this in the latest commit. Thanks.

bkaur-bkj

@rt4914 sir, as per my trial it works thus approving it from myside, PTAL.

rt4914

@jashasweejena Minor changes suggested. Otherwise LGTM.

rt4914 · 2021-12-10T20:08:08Z

app/src/main/java/org/oppia/android/app/options/AudioLanguageActivityPresenter.kt

@@ -12,7 +14,16 @@ class AudioLanguageActivityPresenter @Inject constructor(private val activity: A
  private lateinit var prefSummaryValue: String

  fun handleOnCreate(prefKey: String, prefValue: String) {
-    activity.setContentView(R.layout.audio_language_activity)
+    val binding = AudioLanguageActivityBinding.inflate(activity.layoutInflater, null, false)


Instead of this, use DataBindingUtil to use setContentView

Refactored this to use DataBindingUtil and pushed the code.

Thanks

app/src/main/res/layout/audio_language_activity.xml

rt4914 · 2021-12-10T20:11:01Z

@jashasweejena Please merge with latest develop too.

…g.inflate function

jashasweejena · 2021-12-11T18:54:09Z

Done with the changes. PTAL @rt4914.

Thanks

oppiabot · 2021-12-11T18:57:12Z

Unassigning @jashasweejena since a re-review was requested. @jashasweejena, please make sure you have addressed all review comments. Thanks!

rt4914

LGTM, thanks.

oppiabot · 2021-12-11T19:05:43Z

Unassigning @rt4914 since they have already approved the PR.

BenHenning · 2021-12-17T05:47:24Z

Hi. As of today, some main reviewers have taken time off for the next few weeks, so it may take a little while before we can look at this PR. We appreciate your patience while some of our team members recharge. We'll be fully returning on 4 January 2021.

oppiabot · 2021-12-24T06:10:15Z

Hi @jashasweejena, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

jashasweejena added 4 commits November 18, 2021 01:35

AudioLanguageActivity: Add AppBar into audio_language_activity.xml an…

cd77889

…d set click listeners in AudioLanguageActivityPresenter

AudioLanguageFragment: Remove references to audio_language_toolbar fr…

66e3df1

…om xml and Presenter

AudioLanguageFragment: Remove redundant audio_language_fragment files…

82e9ee8

… and use resolution specific dimens.xml to separate dimensions

Merge branch 'develop' into merge-audio-language-fragment

9caccbd

jashasweejena requested a review from rt4914 as a code owner November 18, 2021 18:19

jashasweejena mentioned this pull request Nov 18, 2021

Merge audio_language_fragment into single xml file #3462

Closed

rt4914 self-assigned this Nov 24, 2021

rt4914 assigned bkaur-bkj and unassigned rt4914 Nov 26, 2021

rt4914 suggested changes Nov 26, 2021

View reviewed changes

app/src/main/java/org/oppia/android/app/options/AudioLanguageActivityPresenter.kt Outdated Show resolved Hide resolved

app/src/main/java/org/oppia/android/app/options/AudioLanguageActivityPresenter.kt Show resolved Hide resolved

rt4914 assigned jashasweejena Nov 26, 2021

AudioLanguageActivity: Migrate to data binding

d15286a

bkaur-bkj assigned rt4914 and unassigned bkaur-bkj Dec 2, 2021

oppiabot bot unassigned jashasweejena Dec 2, 2021

AudioLanguageActivity: Reformat imports to fix KtLint's warnings

926e7b5

AudioLanguageFragmentPresenter: Refactor imports to fix KtLint's warn…

e133ac0

…ings

rt4914 assigned bkaur-bkj Dec 7, 2021

bkaur-bkj reviewed Dec 7, 2021

View reviewed changes

audio_language_activity.xml: Add newline at end to fix warning

67ebdbc

rt4914 removed their assignment Dec 8, 2021

bkaur-bkj approved these changes Dec 9, 2021

View reviewed changes

bkaur-bkj assigned rt4914 and unassigned bkaur-bkj Dec 9, 2021

rt4914 suggested changes Dec 10, 2021

View reviewed changes

rt4914 assigned jashasweejena and unassigned rt4914 Dec 10, 2021

jashasweejena added 3 commits December 12, 2021 00:05

audio_language_activity.xml: Remove unwanted extra lines

82deaad

AudioLanguageActivityPresenter: Use DataBindingUtil instead of bindin…

d86ebd5

…g.inflate function

Merge branch 'develop' into merge-audio-language-fragment

75adf10

oppiabot bot assigned rt4914 and unassigned jashasweejena Dec 11, 2021

rt4914 approved these changes Dec 11, 2021

View reviewed changes

oppiabot bot unassigned rt4914 Dec 11, 2021

oppiabot bot added the PR: LGTM label Dec 11, 2021

oppiabot bot added the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Dec 24, 2021

rt4914 merged commit abe00c8 into oppia:develop Dec 31, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Merge multiple versions of audio_language_fragment.xml #3997

Merge multiple versions of audio_language_fragment.xml #3997

jashasweejena commented Nov 18, 2021

rt4914 commented Nov 24, 2021

jashasweejena commented Nov 24, 2021

rt4914 commented Nov 24, 2021

jashasweejena commented Nov 24, 2021 •

edited

Loading

rt4914 left a comment

jashasweejena commented Nov 26, 2021

bkaur-bkj commented Nov 27, 2021

jashasweejena commented Nov 27, 2021

bkaur-bkj commented Dec 2, 2021

bkaur-bkj commented Dec 2, 2021

bkaur-bkj commented Dec 2, 2021

bkaur-bkj commented Dec 2, 2021

jashasweejena commented Dec 2, 2021

jashasweejena commented Dec 2, 2021

oppiabot bot commented Dec 2, 2021

jashasweejena commented Dec 4, 2021

rt4914 commented Dec 7, 2021

bkaur-bkj Dec 7, 2021

jashasweejena Dec 7, 2021

bkaur-bkj left a comment

rt4914 left a comment

rt4914 Dec 10, 2021

jashasweejena Dec 11, 2021

rt4914 commented Dec 10, 2021

jashasweejena commented Dec 11, 2021

oppiabot bot commented Dec 11, 2021

rt4914 left a comment

oppiabot bot commented Dec 11, 2021

BenHenning commented Dec 17, 2021

oppiabot bot commented Dec 24, 2021

Merge multiple versions of audio_language_fragment.xml #3997

Merge multiple versions of audio_language_fragment.xml #3997

Conversation

jashasweejena commented Nov 18, 2021

Explanation

Essential Checklist

For UI-specific PRs only

Screenshots

rt4914 commented Nov 24, 2021

jashasweejena commented Nov 24, 2021

rt4914 commented Nov 24, 2021

jashasweejena commented Nov 24, 2021 • edited Loading

rt4914 left a comment

Choose a reason for hiding this comment

jashasweejena commented Nov 26, 2021

bkaur-bkj commented Nov 27, 2021

jashasweejena commented Nov 27, 2021

bkaur-bkj commented Dec 2, 2021

bkaur-bkj commented Dec 2, 2021

bkaur-bkj commented Dec 2, 2021

bkaur-bkj commented Dec 2, 2021

jashasweejena commented Dec 2, 2021

jashasweejena commented Dec 2, 2021

oppiabot bot commented Dec 2, 2021

jashasweejena commented Dec 4, 2021

rt4914 commented Dec 7, 2021

bkaur-bkj Dec 7, 2021

Choose a reason for hiding this comment

jashasweejena Dec 7, 2021

Choose a reason for hiding this comment

bkaur-bkj left a comment

Choose a reason for hiding this comment

rt4914 left a comment

Choose a reason for hiding this comment

rt4914 Dec 10, 2021

Choose a reason for hiding this comment

jashasweejena Dec 11, 2021

Choose a reason for hiding this comment

rt4914 commented Dec 10, 2021

jashasweejena commented Dec 11, 2021

oppiabot bot commented Dec 11, 2021

rt4914 left a comment

Choose a reason for hiding this comment

oppiabot bot commented Dec 11, 2021

BenHenning commented Dec 17, 2021

oppiabot bot commented Dec 24, 2021

jashasweejena commented Nov 24, 2021 •

edited

Loading