Fixes #1171 #1173 #1175 #1177: Exploration/Question player high-fi work #1531

MohamedMedhat1998 · 2020-07-27T12:11:13Z

Explanation

This PR implements the high-fi work for Tablet Devices and Split Screen for the following items

What was changed

StateRecyclerViewAssembler

Used the observable field isSplitView for all ViewModels
Added isExtraInteractionAnswerCorrect that is used in the split-screen to center the correct answer of the user on the right

layout-land

Deleted all the layout-land files for the interactions because they didn't exist before split-screen work

Player sub-components

Updated the margins to follow the following rules
- QuestionPlayer sub-components items are centered (128 start and end margin in portrait) (192 start and end margins in landscape)
- ExplorationPlayer sub-components items follow the conversational view (176 and 208 margins in landscape) (112 and 144 margins in portrait)

dimen.xml

Added some values

state_fragment

Updated the margins of the AudioBar and Congratulation text

question_fragment

Updated the margins of the QuestionProgressBar and Congratulation text

Screenshots

Tablet-Land

Tablet-Port

Split-screen

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 made from a branch that is up-to-date with "develop".
The PR's branch is based on "develop" and not on any other branch.
The PR is assigned to an appropriate reviewer in both the Assignees and the Reviewers sections.

…nts-lowfi # Conflicts (Resolved): # app/src/main/java/org/oppia/app/player/state/StatePlayerRecyclerViewAssembler.kt # app/src/main/java/org/oppia/app/player/state/itemviewmodel/FeedbackViewModel.kt # app/src/main/java/org/oppia/app/player/state/itemviewmodel/FractionInteractionViewModel.kt # app/src/main/java/org/oppia/app/player/state/itemviewmodel/InteractionViewModelModule.kt # app/src/main/java/org/oppia/app/player/state/itemviewmodel/NumericInputViewModel.kt # app/src/main/java/org/oppia/app/player/state/itemviewmodel/PreviousButtonViewModel.kt # app/src/main/java/org/oppia/app/player/state/itemviewmodel/ReplayButtonViewModel.kt # app/src/main/java/org/oppia/app/player/state/itemviewmodel/SubmittedAnswerViewModel.kt # app/src/main/java/org/oppia/app/player/state/itemviewmodel/TextInputViewModel.kt # app/src/main/java/org/oppia/app/testing/InputInteractionViewTestActivity.kt # app/src/main/res/layout/question_player_feedback_item.xml

…yer-highfi # Conflicts (Resolved): # app/src/main/java/org/oppia/app/player/state/StatePlayerRecyclerViewAssembler.kt # app/src/main/java/org/oppia/app/player/state/itemviewmodel/ContentViewModel.kt -Some code reduction

# Conflicts (Resolved): # app/src/main/java/org/oppia/app/databinding/StateAssemblerMarginBindingAdapters.kt # app/src/main/java/org/oppia/app/databinding/StateAssemblerPaddingBindingAdapters.kt # app/src/main/java/org/oppia/app/player/state/StatePlayerRecyclerViewAssembler.kt # app/src/main/java/org/oppia/app/player/state/itemviewmodel/ContentViewModel.kt # app/src/main/res/layout-sw600dp-land/content_item.xml # app/src/main/res/layout-sw600dp-port/content_item.xml # app/src/main/res/layout/content_item.xml -continue_interaction_item high-fi

rt4914

Its a nice implementation but there are couple of changes:

The question player is slightly following the conversation layout, which is correct, it should follow the layout similar to mobile question player mocks (not in terms of exact margins but a simple non-conversation view). You can also see this here: https://xd.adobe.com/view/d405de00-a871-4f0f-73a0-f8acef30349b-a234/screen/f136ee72-dd2e-42cb-8ff3-7b1cb6d4eb04/ where it has been mentioned that the marginStart and marginEnd is same for all items.
In this image: https://user-images.githubusercontent.com/8533363/88539787-2a8b9180-d012-11ea-9146-7b84273f5620.png you can notice that the buttons are not correctly aligned.
In this image: https://user-images.githubusercontent.com/8533363/88539832-35462680-d012-11ea-916e-4682b6a1241b.png
The interaction should get aligned with button on the right side. (Mobile Mock reference which shows that letf/right margins are same: https://xd.adobe.com/view/ee9e607b-dbd6-4372-48dc-b687d32af3da-98af/screen/336e968c-0811-4a5b-aac2-1f649ca7983b/specs/)
Similarly in this, https://user-images.githubusercontent.com/8533363/88540095-aede1480-d012-11ea-9331-409d3f2d18e6.png , the left/right margin of Previous Responses Header should also be aligned with that of button.

In short the start/end margin for all items in question player will be same for all items which follows match_parent as width.

Also, I really appreciate you taking some improvisations wherever the mocks were not finished or their were inconsistencies.

MohamedMedhat1998 · 2020-07-28T08:24:32Z

Its a nice implementation but there are couple of changes:

1. The question player is slightly following the conversation layout, which is correct, it should follow the layout similar to mobile question player mocks (not in terms of exact margins but a simple non-conversation view). You can also see this  here: https://xd.adobe.com/view/d405de00-a871-4f0f-73a0-f8acef30349b-a234/screen/f136ee72-dd2e-42cb-8ff3-7b1cb6d4eb04/ where it has been mentioned that the marginStart and marginEnd is same for all items.

2. In this image: https://user-images.githubusercontent.com/8533363/88539787-2a8b9180-d012-11ea-9146-7b84273f5620.png you can notice that the buttons are not correctly aligned.

3. In this image: https://user-images.githubusercontent.com/8533363/88539832-35462680-d012-11ea-916e-4682b6a1241b.png
   The interaction should get aligned with button on the right side. (Mobile Mock reference which shows that letf/right margins are same: https://xd.adobe.com/view/ee9e607b-dbd6-4372-48dc-b687d32af3da-98af/screen/336e968c-0811-4a5b-aac2-1f649ca7983b/specs/)

4. Similarly in this, https://user-images.githubusercontent.com/8533363/88540095-aede1480-d012-11ea-9331-409d3f2d18e6.png , the left/right margin of `Previous Responses Header` should also be aligned with that of button.

In short the start/end margin for all items in question player will be same for all items which follows match_parent as width.

Also, I really appreciate you taking some improvisations wherever the mocks were not finished or their were inconsistencies.

Thanks a lot.
I have worked on the changes, PTAL.

rt4914 · 2020-07-28T18:23:36Z

@MohamedMedhat1998 There is still one alignment issue in the implementation in exploration player:

The right-side alignment of button with the interaction is incorrect. The right side white border should be aligned with the button right end. In landscape as well as portrait.

![Screenshot_1595960414](https://user-images.githubusercontent.com/9396084/88706110-0ae98b80-d12e-11ea-97ae-9e17d0e3f30f.png) ![Screenshot_1595960440](https://user-images.githubusercontent.com/9396084/88706113-0b822200-d12e-11ea-9c20-3f34aab47750.png)

rt4914

LGTM, thanks.

BenHenning · 2020-07-29T16:16:03Z

Out of curiosity, @MohamedMedhat1998 why are we overwriting layout_height as a binding adapter?

MohamedMedhat1998 · 2020-07-29T16:18:48Z

Out of curiosity, @MohamedMedhat1998 why are we overwriting layout_height as a binding adapter?

@BenHenning because the thickness of the previous_responses_header differs based on some conditions.

BenHenning · 2020-07-29T16:24:09Z

I'm really hesitant to override such a core property since the order in which data-binding properties are resolved are quite complicated (leading situations like #1536). We might actually have other subtle crashes/issues with this override--could we use a custom view to set the response header thickness, instead?

MohamedMedhat1998 and others added 27 commits July 25, 2020 17:21

-prepared the player for high-fi work

fb95a1e

Margin / Padding data binding

6f51af5

-Followed the suggestion in the PR

b5671f6

-Fixed lint issues

62b0dfa

-Retrieved deleted by mistake files

668c08d

-content_item high-fi

9e7514f

-continue_navigation_button_item high-fi

8c17ad8

-drag_drop_interaction_item high-fi

b812765

-feedback_item high-fi

e005174

-fraction_interaction_item high-fi

268e153

-next_button_item high-fi

94f52de

-numeric_input_interaction_item high-fi

2c2e65d

-previous_button_item high-fi

5dd1378

-previous_responses_header_item high-fi

9d2f3e5

-replay_button_item high-fi

cb4155d

-return_to_topic_button_item high-fi

9248e29

-selection_interaction_item high-fi

f59c164

-submit_button_item high-fi

263197d

-submitted_answer_item high-fi

ee78692

-text_input_interaction_item high-fi

2fd2693

-Finished all sub-components high-fi work

475e338

-Finished audio bar high-fi work

e6ee453

-Finished all player high-fi work

820f8e7

-Fixed some paddings

ef5a194

MohamedMedhat1998 requested a review from rt4914 July 27, 2020 12:11

MohamedMedhat1998 assigned rt4914 Jul 27, 2020

-restored landscape files

a464d46

-removed some unnecessary changes

3b6edbc

rt4914 suggested changes Jul 28, 2020

View reviewed changes

rt4914 assigned MohamedMedhat1998 and unassigned rt4914 Jul 28, 2020

MohamedMedhat1998 added 3 commits July 28, 2020 09:07

Merge remote-tracking branch 'origin/develop' into player-highfi

c92bb0b

Merge remote-tracking branch 'origin/develop' into player-highfi

cf45108

-Updated the margins in QuestionPlayer

62ad48d

MohamedMedhat1998 assigned rt4914 and unassigned MohamedMedhat1998 Jul 28, 2020

-some nit changes

03346b1

rt4914 assigned MohamedMedhat1998 and unassigned rt4914 Jul 28, 2020

-Fixed margins issues

213cdf6

MohamedMedhat1998 assigned rt4914 and unassigned MohamedMedhat1998 Jul 28, 2020

-Fixed lint issues

a53adca

rt4914 approved these changes Jul 28, 2020

View reviewed changes

rt4914 merged commit 6b1a017 into develop Jul 28, 2020

rt4914 deleted the player-highfi branch July 28, 2020 21:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #1171 #1173 #1175 #1177: Exploration/Question player high-fi work #1531

Fixes #1171 #1173 #1175 #1177: Exploration/Question player high-fi work #1531

MohamedMedhat1998 commented Jul 27, 2020

rt4914 left a comment

MohamedMedhat1998 commented Jul 28, 2020

rt4914 commented Jul 28, 2020 •

edited

Loading

rt4914 left a comment

BenHenning commented Jul 29, 2020

MohamedMedhat1998 commented Jul 29, 2020

BenHenning commented Jul 29, 2020

Fixes #1171 #1173 #1175 #1177: Exploration/Question player high-fi work #1531

Fixes #1171 #1173 #1175 #1177: Exploration/Question player high-fi work #1531

Conversation

MohamedMedhat1998 commented Jul 27, 2020

Explanation

What was changed

StateRecyclerViewAssembler

layout-land

Player sub-components

dimen.xml

state_fragment

question_fragment

Screenshots

Tablet-Land

Tablet-Port

Split-screen

Checklist

rt4914 left a comment

Choose a reason for hiding this comment

MohamedMedhat1998 commented Jul 28, 2020

rt4914 commented Jul 28, 2020 • edited Loading

rt4914 left a comment

Choose a reason for hiding this comment

BenHenning commented Jul 29, 2020

MohamedMedhat1998 commented Jul 29, 2020

BenHenning commented Jul 29, 2020

rt4914 commented Jul 28, 2020 •

edited

Loading