Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #3236: Hi-Fi Add Profile Tablet #3710

Closed
wants to merge 2 commits into from
Closed

Fix #3236: Hi-Fi Add Profile Tablet #3710

wants to merge 2 commits into from

Conversation

viktoriias
Copy link
Contributor

@viktoriias viktoriias commented Aug 20, 2021

Explanation

Fixes #3236: Hi-Fi Add Profile Tablet:

  • Remove layout-land file
  • Introduce dimensions resources

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.

@viktoriias viktoriias marked this pull request as draft August 20, 2021 02:31
@viktoriias viktoriias changed the title Merge mobile files. Hi-Fi Add Profile Tablet Aug 20, 2021
@viktoriias
Copy link
Contributor Author

Portrait Phone

Before After

@viktoriias
Copy link
Contributor Author

Landscape Phone

Before After

@viktoriias viktoriias changed the title Hi-Fi Add Profile Tablet Fix #3236: Hi-Fi Add Profile Tablet Aug 20, 2021
@viktoriias viktoriias marked this pull request as ready for review August 20, 2021 19:54
@viktoriias
Copy link
Contributor Author

@rt4914 Please review.

Copy link
Contributor

@rt4914 rt4914 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@viktoriias The issue is related to two things:

  1. Creating a single xml file instead of multiple files.
  2. Creating Highfi for Tablet mocks too.

Currently this PR is doing 1st task only.

Also the approach mentioned in this PR has been to keep the margins fixed and the content width to be variable. But instead in general we are shifting towards keeping the content width fixed and margins to be variable. You can notice this in mocks too. For example: If you open mobile-portrait and mobile-landscape mocks you will notice that maximum width of items is 304dp which means that irrespective of screen width we are keeping it 304dp width. And for tablet-portrait and tablet-landscape this is 360dp width.

So basically you just need to creating a container which holds all views in it correctly and based on which device/configuration it is you need to control the width of the container to be either 304dp or 360dp.

@rt4914 rt4914 assigned viktoriias and unassigned rt4914 Aug 23, 2021
@viktoriias viktoriias marked this pull request as draft August 24, 2021 21:50
@viktoriias
Copy link
Contributor Author

@viktoriias The issue is related to two things:

  1. Creating a single xml file instead of multiple files.
  2. Creating Highfi for Tablet mocks too.

Currently this PR is doing 1st task only.

Also the approach mentioned in this PR has been to keep the margins fixed and the content width to be variable. But instead in general we are shifting towards keeping the content width fixed and margins to be variable. You can notice this in mocks too. For example: If you open mobile-portrait and mobile-landscape mocks you will notice that maximum width of items is 304dp which means that irrespective of screen width we are keeping it 304dp width. And for tablet-portrait and tablet-landscape this is 360dp width.

So basically you just need to creating a container which holds all views in it correctly and based on which device/configuration it is you need to control the width of the container to be either 304dp or 360dp.

I created a separate PR: #3724 - PTAL

@oppiabot
Copy link

oppiabot bot commented Aug 31, 2021

Hi @viktoriias, 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 3 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!

@oppiabot oppiabot bot added the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Aug 31, 2021
@viktoriias viktoriias closed this Aug 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Corresponds to items that haven't seen a recent update and may be automatically closed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hi-Fi Add Profile Tablet
3 participants