Skip to content

fix-next(tabview): fix tab title layout in modal root tabview Fixes #5392 #5435

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

Merged
merged 3 commits into from
Mar 2, 2018

Conversation

manoldonev
Copy link
Contributor

@manoldonev manoldonev commented Feb 19, 2018

Fixes #5392

See comments below.

In a modal dialog scenario with root tabview inside, the DialogFragment content at some point is set via PhoneWindow.setContentView(view) call where view is the gridLayout root element of the tabview. This results in setContentView(view, new ViewGroup.LayoutParams(MATCH_PARENT, MATCH_PARENT)) call that unconditionally overwrites the default instance of CommonLayoutParams attached to the gridLayout by us. As the default setter essentially created new ViewGroup.LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.MATCH_PARENT, Gravity.FILL) we lose the Gravity.FILL setting and that prevents the gridLayout from stretching horizontally so tab item titles do not distribute the excess whitespace among themselves.

The proposed solution is to add a no-op outer native wrapper element to be laid out by the DialogFragment so we can perform the layout of the inner gridLayout element as expected.

Alternatively (as this problem is related only to modal dialog scenarios and I do not like the solution very much) we can make no changes to the tabview codebase and instruct those devs that hit the problem to add the additional GridLayout wrapper element by themselves in their own application code (the drawback is that it looks "broken" out-of-the-box).

Also, @MartoYankov pointed out something else that we might consider at some point -- generally the problem here is triggered by https://github.com/NativeScript/tns-core-modules-widgets/blob/a9ac20da20422ae57aa4cfa76bc03dc27d6bc885/android/widgets/src/main/java/org/nativescript/widgets/GridLayout.java#L259 in the GridLayout codebase that decides how the grid layout should behave based on a setting of its own LayoutParams while Android documentation states that "LayoutParams are used by views to tell their parents how they want to be laid out".

BREAKING CHANGE: [Android] NativeScript no longer overwrites the horizontal/vertical alignment on the user-defined root visual element when opening it in modal dialog window (i.e. not fullscreen).

If your application logic relied on the previous behavior you need to manually set horizontalAlignment="center" and verticalAlignment="middle" on the root visual element you are showing modally.

@manoldonev manoldonev added this to the 4.0 milestone Feb 19, 2018
@manoldonev manoldonev self-assigned this Feb 19, 2018
@ghost ghost added the in progress label Feb 19, 2018
@manoldonev manoldonev force-pushed the mdonev/modal-tab-title-layout-fix branch from 9261ba9 to aed1044 Compare February 19, 2018 13:03
@vakrilov
Copy link
Contributor

Another approach that might worth considering: Doing the wrap with the additional gird layout in the DialogFragment when showing the dialog (somewhere here). This will probably prevent similar problems with other views hosted inside a dialog fragment.

@manoldonev manoldonev force-pushed the mdonev/modal-tab-title-layout-fix branch from aed1044 to 1ff9bb7 Compare February 22, 2018 16:05
@hshristov
Copy link
Contributor

@manoldonev manoldonev force-pushed the mdonev/modal-tab-title-layout-fix branch from 0b140c6 to 4af6e72 Compare February 26, 2018 11:50
@manoldonev
Copy link
Contributor Author

manoldonev commented Feb 26, 2018

Thanks for the tip @hshristov 👍

Indeed those explicit alignment setters are the root cause of the problem and not PhoneWindow.setContentView(view) as I thought initially.

@manoldonev manoldonev force-pushed the mdonev/modal-tab-title-layout-fix branch from d175deb to 58535fe Compare February 26, 2018 17:29
<Page xmlns="http://schemas.nativescript.org/tns.xsd" shownModally="onShownModally" loaded="onLoaded" unloaded="onUnloaded" backgroundColor="Red">
<Page xmlns="http://schemas.nativescript.org/tns.xsd" shownModally="onShownModally"
loaded="onLoaded" unloaded="onUnloaded" backgroundColor="Red"
horizontalAlignment="center" verticalAlignment="middle">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change is required as we no longer force alignment settings on user content unless it is absolutely necessary i.e. if fullscreen dialog is requested we would override these and stretch the user content.

@manoldonev manoldonev force-pushed the mdonev/modal-tab-title-layout-fix branch from 58535fe to 06b634d Compare February 28, 2018 15:31
@manoldonev manoldonev merged commit 9214883 into master Mar 2, 2018
@ghost ghost removed bug in progress labels Mar 2, 2018
@manoldonev manoldonev deleted the mdonev/modal-tab-title-layout-fix branch March 2, 2018 11:27
@lock
Copy link

lock bot commented Aug 26, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators Aug 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TabView] Tabitem title not measured/arranged properly in a modal dialog with root tabview inside
4 participants