Fixes part of #40 & #42: Button extreme margins #3192

rt4914 · 2021-05-13T13:07:27Z

Explanation

Fixes part of #40
Fixes part of #42

This PR introduces the dimens value for start/end margins for buttons which will be used in later PRs.

Also, check below comments where I have mentioned reference for each value.

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.

rt4914 · 2021-05-13T13:07:50Z

app/src/main/res/values/dimens.xml

+  <dimen name="general_button_item_exploration_split_view_margin_start">48dp</dimen>
+  <dimen name="general_button_item_exploration_split_view_margin_end">48dp</dimen>
+
+  <!-- General Button: Exploration View -->


https://xd.adobe.com/view/dfe8ccd4-39eb-414d-a0b2-b5674357f966-8abb/screen/3c9aff8d-2319-4570-818e-cbef229a7419/specs/

rt4914 · 2021-05-13T13:08:15Z

app/src/main/res/values/dimens.xml

+  <dimen name="general_button_item_question_split_view_margin_start">48dp</dimen>
+  <dimen name="general_button_item_question_split_view_margin_end">48dp</dimen>
+
+  <!-- General Button: Question View -->


https://xd.adobe.com/view/dfe8ccd4-39eb-414d-a0b2-b5674357f966-8abb/screen/b6d8fedf-85aa-4052-9b7a-5d6be7f03e01/specs/

rt4914 · 2021-05-13T13:09:28Z

app/src/main/res/values/dimens.xml

+  <dimen name="general_button_item_exploration_view_margin_start">28dp</dimen>
+  <dimen name="general_button_item_exploration_view_margin_end">28dp</dimen>
+
+  <!-- General Button: Question Split View -->


https://xd.adobe.com/view/dfe8ccd4-39eb-414d-a0b2-b5674357f966-8abb/screen/f97ba4a6-b92f-4388-b1bd-0a80f49d552d/specs/

rt4914 · 2021-05-13T13:09:59Z

app/src/main/res/values/dimens.xml

@@ -399,4 +399,20 @@
  <!-- Previous Responses Item: Question View -->
  <dimen name="previous_responses_item_question_view_margin_start">44dp</dimen>
  <dimen name="previous_responses_item_question_view_margin_end">44dp</dimen>
+
+  <!-- General Button Item: Exploration Split View -->


https://xd.adobe.com/view/dfe8ccd4-39eb-414d-a0b2-b5674357f966-8abb/screen/7d089aae-3288-4474-8e4b-14a6f67878f3/specs/

The same applies to here, this is the mobile res directory so we might not be using the split resources here.

Actually Sliptview depends on two things: (a.) Physical size of more than 7" and (b.) DragAndDrop/ImageRegionSelection interaction

It is independent of tablet or mobile or its configuration

So there might come a mobile which is more than 7" and in that case will need to use split screen.

rt4914 · 2021-05-13T13:10:20Z

app/src/main/res/values-sw600dp-port/dimens.xml

+  <dimen name="general_button_item_exploration_split_view_margin_start">48dp</dimen>
+  <dimen name="general_button_item_exploration_split_view_margin_end">48dp</dimen>
+
+  <!-- General Button: Exploration View -->


https://xd.adobe.com/view/dfe8ccd4-39eb-414d-a0b2-b5674357f966-8abb/screen/986ffd25-9e86-42a2-b870-aa6e754799e4/specs/

Do we need these values?

As this is a port res dir, so if they will always use the split view, the split resources will be used. I think we can remove these items!! Let me know if I miss anything here.

Actually Sliptview depends on two things: (a.) Physical size of more than 7" and (b.) DragAndDrop/ImageRegionSelection interaction

It is independent of tablet or mobile or its configuration

rt4914 · 2021-05-13T13:10:32Z

app/src/main/res/values-sw600dp-port/dimens.xml

+  <dimen name="general_button_item_question_split_view_margin_start">48dp</dimen>
+  <dimen name="general_button_item_question_split_view_margin_end">48dp</dimen>
+
+  <!-- General Button: Question View -->


https://xd.adobe.com/view/dfe8ccd4-39eb-414d-a0b2-b5674357f966-8abb/screen/243dda5c-50b8-4666-b3f2-22bbd7e6023a/specs/

rt4914 · 2021-05-13T13:10:41Z

app/src/main/res/values-sw600dp-port/dimens.xml

@@ -200,4 +200,20 @@
  <!-- Previous Responses Item: Question View -->
  <dimen name="previous_responses_item_question_view_margin_start">140dp</dimen>
  <dimen name="previous_responses_item_question_view_margin_end">140dp</dimen>
+
+  <!-- General Button Item: Exploration Split View -->


https://xd.adobe.com/view/dfe8ccd4-39eb-414d-a0b2-b5674357f966-8abb/screen/7d089aae-3288-4474-8e4b-14a6f67878f3/specs/

rt4914 · 2021-05-13T13:10:47Z

app/src/main/res/values-sw600dp-port/dimens.xml

+  <dimen name="general_button_item_exploration_view_margin_start">112dp</dimen>
+  <dimen name="general_button_item_exploration_view_margin_end">112dp</dimen>
+
+  <!-- General Button: Question Split View -->


https://xd.adobe.com/view/dfe8ccd4-39eb-414d-a0b2-b5674357f966-8abb/screen/f97ba4a6-b92f-4388-b1bd-0a80f49d552d/specs/

rt4914 · 2021-05-13T13:11:29Z

app/src/main/res/values-sw600dp-land/dimens.xml

+  <dimen name="general_button_item_exploration_split_view_margin_start">48dp</dimen>
+  <dimen name="general_button_item_exploration_split_view_margin_end">64dp</dimen>
+
+  <!-- General Button: Exploration View -->


https://xd.adobe.com/view/dfe8ccd4-39eb-414d-a0b2-b5674357f966-8abb/screen/9b11d6ac-ba50-4d28-b355-28916363cfc9/specs/

Looks a bit odd, the start margin is the same as the content, which makes them display in a similar margin of 176, but the end, the button has 176 and the content has 208.

But, in another exploration view, we tend to keep the button more margin so it comes inside than content.

For mobile we are generally keeping the buttons a bit inward w.r.t. overall content but in tablet we are keeping them outward. The outward is more correct but for mobile we need to make sure that there is sufficient margins on all side and thats why we kept it inwards.

rt4914 · 2021-05-13T13:11:41Z

app/src/main/res/values-land/dimens.xml

+  <dimen name="general_button_item_exploration_split_view_margin_start">48dp</dimen>
+  <dimen name="general_button_item_exploration_split_view_margin_end">64dp</dimen>
+
+  <!-- General Button: Exploration View -->


https://xd.adobe.com/view/dfe8ccd4-39eb-414d-a0b2-b5674357f966-8abb/screen/8b4be174-68ae-4b99-b937-9f517c6799d6/specs/

rt4914 · 2021-05-13T13:11:56Z

app/src/main/res/values-land/dimens.xml

+  <dimen name="general_button_item_question_split_view_margin_start">48dp</dimen>
+  <dimen name="general_button_item_question_split_view_margin_end">64dp</dimen>
+
+  <!-- General Button: Question View -->


https://xd.adobe.com/view/dfe8ccd4-39eb-414d-a0b2-b5674357f966-8abb/screen/acb4086c-cdd8-4d4d-a909-a2d18e1472fc/specs/

rt4914 · 2021-05-13T13:12:07Z

app/src/main/res/values-sw600dp-land/dimens.xml

+  <dimen name="general_button_item_question_split_view_margin_start">48dp</dimen>
+  <dimen name="general_button_item_question_split_view_margin_end">64dp</dimen>
+
+  <!-- General Button: Question View -->


https://xd.adobe.com/view/dfe8ccd4-39eb-414d-a0b2-b5674357f966-8abb/screen/01adc357-dec8-4f7a-b639-a9aafa89f3f3/specs/

rt4914 · 2021-05-13T13:12:16Z

app/src/main/res/values-sw600dp-land/dimens.xml

@@ -197,4 +197,20 @@
  <!-- Previous Responses Item: Question View -->
  <dimen name="previous_responses_item_question_view_margin_start">204dp</dimen>
  <dimen name="previous_responses_item_question_view_margin_end">204dp</dimen>
+
+  <!-- General Button Item: Exploration Split View -->


https://xd.adobe.com/view/dfe8ccd4-39eb-414d-a0b2-b5674357f966-8abb/screen/60ac31c1-6ac5-4a05-b79c-8702a095cc39/specs/

rt4914 · 2021-05-13T13:12:22Z

app/src/main/res/values-land/dimens.xml

@@ -149,4 +149,20 @@
  <!-- Previous Responses Item: Question View -->
  <dimen name="previous_responses_item_question_view_margin_start">76dp</dimen>
  <dimen name="previous_responses_item_question_view_margin_end">76dp</dimen>
+
+  <!-- General Button Item: Exploration Split View -->


https://xd.adobe.com/view/dfe8ccd4-39eb-414d-a0b2-b5674357f966-8abb/screen/60ac31c1-6ac5-4a05-b79c-8702a095cc39/specs/

rt4914 · 2021-05-13T13:12:28Z

app/src/main/res/values-land/dimens.xml

+  <dimen name="general_button_item_exploration_view_margin_start">64dp</dimen>
+  <dimen name="general_button_item_exploration_view_margin_end">64dp</dimen>
+
+  <!-- General Button: Question Split View -->


https://xd.adobe.com/view/dfe8ccd4-39eb-414d-a0b2-b5674357f966-8abb/screen/a32ccabf-4ae8-4112-a021-333f0d0466d6/specs/

rt4914 · 2021-05-13T13:12:34Z

app/src/main/res/values-sw600dp-land/dimens.xml

+  <dimen name="general_button_item_exploration_view_margin_start">176dp</dimen>
+  <dimen name="general_button_item_exploration_view_margin_end">176dp</dimen>
+
+  <!-- General Button: Question Split View -->


https://xd.adobe.com/view/dfe8ccd4-39eb-414d-a0b2-b5674357f966-8abb/screen/a32ccabf-4ae8-4112-a021-333f0d0466d6/specs/

anandwana001

Nit thoughts left, Thanks @rt4914

anandwana001

LGTM

oppiabot · 2021-05-17T16:42:45Z

Unassigning @anandwana001 since they have already approved the PR.

Button extreme margins

3d60833

rt4914 commented May 13, 2021

View reviewed changes

rt4914 requested a review from anandwana001 May 13, 2021 13:13

rt4914 assigned anandwana001 May 13, 2021

anandwana001 suggested changes May 17, 2021

View reviewed changes

anandwana001 assigned rt4914 and unassigned anandwana001 May 17, 2021

rt4914 assigned anandwana001 and unassigned rt4914 May 17, 2021

anandwana001 approved these changes May 17, 2021

View reviewed changes

oppiabot bot unassigned anandwana001 May 17, 2021

oppiabot bot added the PR: LGTM label May 17, 2021

anandwana001 assigned rt4914 May 17, 2021

anandwana001 merged commit 132d333 into develop May 17, 2021

anandwana001 deleted the general-button-extreme-margins branch May 17, 2021 16:44

Fixes part of #40 & #42: Button extreme margins #3192

Fixes part of #40 & #42: Button extreme margins #3192

Conversation

rt4914 commented May 13, 2021

Explanation

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anandwana001 left a comment

Choose a reason for hiding this comment

anandwana001 left a comment

Choose a reason for hiding this comment

oppiabot bot commented May 17, 2021