Merge profile_chooser_fragment.xml into single xml file

[rt4914]

Currently there are 4 versions of profile_chooser_fragment.xml file merge it into single xml file.

We can use https://text-compare.com/ to compare two versions of this file and for all the differences we can create variables in dimens.xml file and use it accordingly.

It can happen that it becomes really hard to merge all 4 files into single file, in such cases prefer code-readability over single file. Therefore it can be fine if in the end the code still contains 2 or 3 files.

Note: In PR, make sure you add before and after screenshot of mobile-portrait, mobile-landscape, tablet-portrait and tablet-landscape for comparison and make sure that there is not difference between before and after UI.

[priyatanu]

@rt4914 , I would like to work on this issue ?

[priyatanu]

@Arjupta , I would like to work on this issue ?

[Arjupta]

@priyatanu assigned it to you

[priyatanu]

Thank you :D

[adizcode]

I'm interested! Is this available?

[priyatanu]

I'm interested! Is this available?

Hi @adizcode , I am working on it.

[adizcode]

Okay

[ShivamMadlani]

I would like to work on this issue. Plz assign it to me

[ritece]

I'm working on this! Please assign.

[rt4914]

Unassigned because of inactivity.

[dmdbilal]

Can i work on this issue ?

[sorinmircea]

I did open: #4729

I do have a question regarding sw600dp-land and port files. The only difference that puzzles me is:

• app:layoutMarginTop="@{hasProfileEverBeenAddedValue ? @dimen/space_0dp: @dimen/profile_chooser_margin_top_profile_not_added}"
• app:layoutMarginTop="@{hasProfileEverBeenAddedValue ? @dimen/profile_chooser_fragment_margin_top_profile_already_added : @dimen/profile_chooser_fragment_margin_top_profile_not_added}"

Is there any way to merge this (@dimen/space_0dp and @dimen/profile_chooser_fragment_margin_top_profile_already_added
being different) into one XML without relying on any code changes?

[BenHenning]

@dmdbilal it looks like this issue is being actively worked on, so you’ll need to find another one to work on.

[BenHenning]

I did open: #4729

I do have a question regarding sw600dp-land and port files. The only difference that puzzles me is:

• app:layoutMarginTop="@{hasProfileEverBeenAddedValue ? @dimen/space_0dp: @dimen/profile_chooser_margin_top_profile_not_added}"
• app:layoutMarginTop="@{hasProfileEverBeenAddedValue ? @dimen/profile_chooser_fragment_margin_top_profile_already_added : @dimen/profile_chooser_fragment_margin_top_profile_not_added}"

Is there any way to merge this (@dimen/space_0dp and @dimen/profile_chooser_fragment_margin_top_profile_already_added being different) into one XML without relying on any code changes?

@msorins is the difference between those two based on a qualifier (like sw600dp or land)? If so, you could make sure that profile_chooser_fragment_margin_top_profile_already_added is defined as 0 in its dimens.xml file corresponding to whichever qualifier is used the 0dp code today. Could that work?

I’m also not sure what is meant by “no code changes”—could you clarify?

[sorinmircea]

Thank you for the tip, I was able to solve this and amended the PR with another commit.

[seanlip]

Deassigning @msorins since he is no longer working on this.

[ArchitJain1201]

@seanlip I would like to work on this issue. Can you please assign it to me?

[seanlip]

@ArchitJain1201 Feel free to go ahead and raise a PR. If it looks reasonable we can assign you to this. Thanks!

[kunsistent]

Can I work on this issue? @seanlip

[seanlip]

@kunsistent My response to @ArchitJain1201 above applies here, too.

[masclot]

I created a PR, #4849, but I cannot assign it. Will it be automatically assigned or do i need to ping somebody?

[seanlip]

Ah, I assigned @rt4914 for you @masclot. In the future I think you can just say "@xxx PTAL" and Oppiabot will assign them for you. Thanks!

[chrislee115]

I'm working on this issue and making a PR

[seanlip]

Hi @masclot @chrislee115 Just a question about this -- reading the conversation above, there's a PR #4849 that's already been merged. Are there remaining tasks for this issue?

/cc @BenHenning @adhiamboperes

[seanlip]

(Also, PR #4849 says that it fixes part of the issue but doesn't really specify which part it fixes or which parts are left ... @masclot would you mind clarifying please? Thanks!)

[masclot]

I merged the two layouts for big screens. There remained two layouts for small screens which I think can be merged. I don't think it makes sense to merge the layout for big screen with the ones for small screens.

[chrislee115]

Are you referring to merging layout-sw600dp-land and layout-sw600dp-port?

[seanlip]

Per discussion with @BenHenning, closing this issue because it looks like merging these files is a bit complicated and probably not worth it.