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

[TIMOB-25173] Android: Modified layouts to behave more like iOS. #9320

Merged
merged 7 commits into from Aug 22, 2017

Conversation

jquick-axway
Copy link
Contributor

@jquick-axway jquick-axway commented Aug 16, 2017

JIRA:
https://jira.appcelerator.org/browse/TIMOB-25173

Also resolves the following JIRA cases...

Summary:

  • Changed Android to resolve Size/Fill layout conflicts like iOS and Windows.
    • Note: Only an issue if parent uses Ti.UI.SIZE and child uses Ti.UI.FILL.
    • Android used to shrink the parent to fit the min size allowed by the child.
    • Now increases the size of the child to fill the parent's parent, like iOS/Windows.
  • Fixed horizontal layouts where views using FILL width would wrongly wrap to the next row.
    • Only an issue if another view is already in the row. (Resulting FILL width was too big.)
  • Fixed layouts to size itself using view's top/left/bottom/right/center properties if width or height property was not set.
    • Only applies if at least 2 coordinate properties were provided.
    • Now matches iOS' behavior.
  • Constrained child view width to parent's max width like iOS when using wrapping horizontal views.

Tests:

- Changed Android to resolve Size/Fill layout conflicts like iOS and Windows.
  * Note: Only an issue if parent uses Ti.UI.SIZE and child uses Ti.UI.FILL.
  * Android used to shrink the parent to fit the min size allowed by the child.
  * Now increases the size of the child to fill the parent's parent, like iOS/Windows.
- Fixed horizontal layouts where views using FILL width would wrongly wrapped to next row.
  * Only an issue if another view is already in the row. (Resulting FILL width was too big.)
- Fixed layouts to prioritize size defined by view's top/left/bottom/right/center properties over FILL and SIZE.
  * Only applies if at least 2 coordinate properties were provided.
  * Now matches iOS' behavior.
- Constrained child view width to parent's max width like iOS when using wrapping horizontal views.
- This commit also resolves the following layout issues:
  * TIMOB-12996
  * TIMOB-15097
  * TIMOB-17628
  * TIMOB-17984
  * TIMOB-19536
  * TIMOB-19598
  * TIMOB-19814
Copy link
Contributor

@garymathews garymathews left a comment

Choose a reason for hiding this comment

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

CR: LGTM

NOTE: QE could you also test the cases in #9123 and #9073

@mukherjee2 mukherjee2 self-requested a review August 16, 2017 18:58
@jquick-axway
Copy link
Contributor Author

There is a new horizontal wrapping bug with views using Ti.UI.SIZE where they are constrained by the parent's remaining width instead of the parent's full width. This was discovered while testing with PR #9073 test case.

We've also discovered an issue where the child view's left/center/right properties are not being used to calculate its width within a horizontal layout. iOS does this, but Android does not. This is not a new bug (existed before my code changes).

I plan on adding fixes for the above to this PR in the near future.

…t/right/top/bottom/center properties to work like iOS.

- Now supports setting view width/height based on position properties relative to parent's remaining space.
- Also fixed horizontal wrapping regression introduced in last commit.
  * Views using Ti.UI.SIZE were wrongly using row's remaing space instead of view's desired width.
@jquick-axway
Copy link
Contributor Author

The 2 issues have been fixed.

@mukherjee2, can you resume testing please?
Also notice that I've added a new HorizontalPinningTest.js test to TIMOB-25173.

Copy link
Contributor

@garymathews garymathews left a comment

Choose a reason for hiding this comment

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

CR: PASS

Copy link
Contributor

@mukherjee2 mukherjee2 left a comment

Choose a reason for hiding this comment

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

Passes FR. Details are in PR 9321.

Copy link
Contributor

@mukherjee2 mukherjee2 left a comment

Choose a reason for hiding this comment

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

Passes FR. Details are in PR 9321.

@mukherjee2 mukherjee2 merged commit e4bd715 into tidev:master Aug 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants