Fix #3572 revision_card_fragment.xml files merged into two xml file #4101

ArchitJain1201 · 2022-01-14T08:57:44Z

Explanation

Fix #3572

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

Before

After

Before

After

Before

After

Before

After

@ptal bkaur-bkj

bkaur-bkj

@ArchitJain1201 also change the title of PR it should be FIx #bugnum

bkaur-bkj · 2022-01-17T05:24:52Z

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

+  <dimen name="revision_card_fragment_layout_margin_end">28dp</dimen>
+  <dimen name="revision_card_fragment_layout_margin_top_button">48dp</dimen>
+
+


@ArchitJain1201 remove this extra line

Ma'am How can I remove revision_card_fragment_layout_margin_top_button as its value is being change in port and land mode and even it carry a different value than revision_card_fragment_layout_margin_top_text. ?

you don't need to remove it, you wont be able to merge 4 files into 1 as tab layout is different than mobile layout so just merge 2 tab layouts into 1 and 2 mobile into one using the dimens files and adding the required values there

changes was made

bkaur-bkj · 2022-01-17T05:25:27Z

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

@@ -593,4 +593,13 @@

  <!-- StoryFragment -->
  <dimen name="story_fragment_padding_bottom">160dp</dimen>
+
+<!--  Revision Card Fragment-->


@ArchitJain1201 write name of file without spacing as StoryFragment is written

changes was made

ArchitJain1201 · 2022-01-18T04:14:35Z

PTAL @bkaur-bkj

oppiabot · 2022-01-18T04:17:38Z

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

bkaur-bkj · 2022-01-18T04:12:54Z

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

-        android:layout_marginTop="48dp"
-        android:layout_marginEnd="28dp"
+        android:layout_marginStart="@dimen/revision_card_fragment_layout_margin"
+        android:layout_marginTop="@dimen/revision_card_fragment_layout_margin_top_button"


@ArchitJain1201 if instead of naming dimension as margin_top_button it will look more proper as buttom_margin_top, PTAL and change it along all the files, also in 1-2 files still the extra lines are not removed

Sorry Ma'am, not able to get you, which extra lines you are talking about?

bkaur-bkj · 2022-01-18T04:19:42Z

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

+  <dimen name="revision_card_fragment_layout_margin_end">28dp</dimen>
+  <dimen name="revision_card_fragment_layout_margin_top_button">48dp</dimen>
+
+


you don't need to remove it, you wont be able to merge 4 files into 1 as tab layout is different than mobile layout so just merge 2 tab layouts into 1 and 2 mobile into one using the dimens files and adding the required values there

bkaur-bkj · 2022-01-18T04:21:15Z

@ArchitJain1201 also the branch has merge conflicts you need to resolve them by updating your branch with develop

ArchitJain1201 · 2022-01-18T04:58:03Z

PTAL @bkaur-bkj

oppiabot · 2022-01-18T05:01:06Z

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

bkaur-bkj · 2022-01-19T05:32:29Z

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

+  <!--  RevisionCardFragment-->
+  <dimen name="revision_card_fragment_layout_min_width" tools:ignore="MissingDefaultResource">512dp</dimen>
+  <dimen name="revision_card_fragment_padding_bottom">160dp</dimen>
+  <dimen name="revision_card_fragment_layout_text_margin_top">36dp</dimen>


This will work but I guess it is not preferred to do this way. cc @rt4914 PTAL

HAve consulted to sir and the required changes were made

rt4914

@ArchitJain1201 Please add before and after screenshots in PR description. Thanks a lot.

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

ArchitJain1201 · 2022-01-19T19:20:43Z

PTAL @bkaur-bkj @rt4914

oppiabot · 2022-01-19T19:23:46Z

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

rt4914 · 2022-01-19T19:36:55Z

@ArchitJain1201 Please reply to all the comments above so that the reviewer can know whether that suggestion was applied or not.

ArchitJain1201 · 2022-01-19T20:36:38Z

PTAL @rt4914 sir I have reply to each and every comment please have a look

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

rt4914

@ArchitJain1201 PTAL thanks.

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

ArchitJain1201 · 2022-01-28T15:16:06Z

PTAL @rt4914

oppiabot · 2022-01-28T15:19:09Z

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

rt4914

@ArchitJain1201 One change is pending.

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

ArchitJain1201 · 2022-01-28T16:21:55Z

PTAL @rt4914

oppiabot · 2022-01-28T16:40:35Z

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

rt4914

LGTM, thanks.

oppiabot · 2022-02-02T07:53:11Z

Unassigning @rt4914 since they have already approved the PR.

oppia#3572 revision_card_fragment.xml files merged into two xml file

eb9c97c

ArchitJain1201 requested a review from rt4914 as a code owner January 14, 2022 08:57

bkaur-bkj suggested changes Jan 17, 2022

View reviewed changes

bkaur-bkj assigned ArchitJain1201 Jan 17, 2022

ArchitJain1201 changed the title ~~#3572 revision_card_fragment.xml files merged into two xml file~~ Fix #3572 revision_card_fragment.xml files merged into two xml file Jan 17, 2022

changed the filename and removed the margin_end variable

2217122

ArchitJain1201 requested a review from bkaur-bkj January 17, 2022 10:57

oppiabot bot unassigned ArchitJain1201 Jan 18, 2022

bkaur-bkj suggested changes Jan 18, 2022

View reviewed changes

bkaur-bkj assigned ArchitJain1201 Jan 18, 2022

ArchitJain1201 added 2 commits January 18, 2022 10:15

Merge branch 'develop' into revision-card-fragment

f986e41

changes varibale names to text/button_margin_top

e07bc98

oppiabot bot unassigned ArchitJain1201 Jan 18, 2022

bkaur-bkj reviewed Jan 19, 2022

View reviewed changes

bkaur-bkj assigned rt4914 Jan 19, 2022

rt4914 suggested changes Jan 19, 2022

View reviewed changes

app/src/main/res/values/dimens.xml Outdated Show resolved Hide resolved

app/src/main/res/values/dimens.xml Outdated Show resolved Hide resolved

rt4914 assigned ArchitJain1201 and unassigned rt4914 Jan 19, 2022

title name and added margin_start/end

5b59827

oppiabot bot assigned rt4914 and unassigned ArchitJain1201 Jan 19, 2022

rt4914 removed their assignment Jan 19, 2022

rt4914 assigned rt4914 and bkaur-bkj Jan 19, 2022

rt4914 unassigned ArchitJain1201 Jan 20, 2022

bkaur-bkj approved these changes Jan 20, 2022

View reviewed changes

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

bkaur-bkj removed their assignment Jan 20, 2022

rt4914 suggested changes Jan 25, 2022

View reviewed changes

app/src/main/res/values-sw600dp-port/dimens.xml Outdated Show resolved Hide resolved

rt4914 assigned ArchitJain1201 and unassigned rt4914 Jan 25, 2022

removed ignore and added ..min_width in dimens.xml

9355f66

oppiabot bot assigned rt4914 and unassigned ArchitJain1201 Jan 28, 2022

rt4914 suggested changes Jan 28, 2022

View reviewed changes

app/src/main/res/values-sw600dp-port/dimens.xml Outdated Show resolved Hide resolved

rt4914 assigned ArchitJain1201 and unassigned rt4914 Jan 28, 2022

named changed to revision_card_fragment_layout_tablet_min_width

710332a

oppiabot bot assigned rt4914 and unassigned ArchitJain1201 Jan 28, 2022

Merge branch 'develop' into revision-card-fragment

2f29422

rt4914 approved these changes Feb 2, 2022

View reviewed changes

rt4914 enabled auto-merge (squash) February 2, 2022 07:50

oppiabot bot unassigned rt4914 Feb 2, 2022

oppiabot bot added the PR: LGTM label Feb 2, 2022

rt4914 assigned ArchitJain1201 Feb 2, 2022

rt4914 merged commit 78e1b91 into oppia:develop Feb 2, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #3572 revision_card_fragment.xml files merged into two xml file #4101

Fix #3572 revision_card_fragment.xml files merged into two xml file #4101

ArchitJain1201 commented Jan 14, 2022 •

edited

bkaur-bkj left a comment

bkaur-bkj Jan 17, 2022

ArchitJain1201 Jan 17, 2022

bkaur-bkj Jan 18, 2022

ArchitJain1201 Jan 19, 2022

bkaur-bkj Jan 17, 2022

ArchitJain1201 Jan 19, 2022

ArchitJain1201 commented Jan 18, 2022

oppiabot bot commented Jan 18, 2022

bkaur-bkj Jan 18, 2022

ArchitJain1201 Jan 18, 2022

bkaur-bkj Jan 18, 2022

bkaur-bkj commented Jan 18, 2022

ArchitJain1201 commented Jan 18, 2022

oppiabot bot commented Jan 18, 2022

bkaur-bkj Jan 19, 2022

ArchitJain1201 Jan 19, 2022

rt4914 left a comment

ArchitJain1201 commented Jan 19, 2022

oppiabot bot commented Jan 19, 2022

rt4914 commented Jan 19, 2022

ArchitJain1201 commented Jan 19, 2022

rt4914 left a comment

ArchitJain1201 commented Jan 28, 2022

oppiabot bot commented Jan 28, 2022

rt4914 left a comment

ArchitJain1201 commented Jan 28, 2022

oppiabot bot commented Jan 28, 2022

rt4914 left a comment

oppiabot bot commented Feb 2, 2022

		<dimen name="revision_card_fragment_layout_margin_end">28dp</dimen>
		<dimen name="revision_card_fragment_layout_margin_top_button">48dp</dimen>

Fix #3572 revision_card_fragment.xml files merged into two xml file #4101

Fix #3572 revision_card_fragment.xml files merged into two xml file #4101

Conversation

ArchitJain1201 commented Jan 14, 2022 • edited

Explanation

Essential Checklist

For UI-specific PRs only

bkaur-bkj left a 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

ArchitJain1201 commented Jan 18, 2022

oppiabot bot commented Jan 18, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bkaur-bkj commented Jan 18, 2022

ArchitJain1201 commented Jan 18, 2022

oppiabot bot commented Jan 18, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rt4914 left a comment

Choose a reason for hiding this comment

ArchitJain1201 commented Jan 19, 2022

oppiabot bot commented Jan 19, 2022

rt4914 commented Jan 19, 2022

ArchitJain1201 commented Jan 19, 2022

rt4914 left a comment

Choose a reason for hiding this comment

ArchitJain1201 commented Jan 28, 2022

oppiabot bot commented Jan 28, 2022

rt4914 left a comment

Choose a reason for hiding this comment

ArchitJain1201 commented Jan 28, 2022

oppiabot bot commented Jan 28, 2022

rt4914 left a comment

Choose a reason for hiding this comment

oppiabot bot commented Feb 2, 2022

ArchitJain1201 commented Jan 14, 2022 •

edited