Tabs styling improvements by vtrifonov · Pull Request #8366 · NativeScript/NativeScript

vtrifonov · 2020-02-21T15:44:51Z

PR Checklist

The PR title follows our guidelines: https://github.com/NativeScript/NativeScript/blob/master/CONTRIBUTING.md#commit-messages.
There is an issue for the bug/feature this PR is for. To avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it.
You have signed the CLA.
All existing tests are passing: https://github.com/NativeScript/NativeScript/blob/master/DevelopmentWorkflow.md#running-unit-tests.
Tests for the changes are included - https://github.com/NativeScript/NativeScript/blob/master/WritingUnitTests.md.

What is the current behavior?

The tabs in iOS are left-aligned and this cannot be modified
The styling of the Tabs TabStrip component is not being applied in ios.
The default value of the TextTransform for the tab strips labels in android is "none" which differs from the Material design guidelines

What is the new behavior?

Now we allow leading, justified, center and centerSelected types of alignment of the Tabs TabStrip items in iOS. To change the behavior set the iOSTabBarItemsAlignment property to the Tabs component to one of the following:
- leading - tab items are aligned to the left
- justified - tab strip is split equally to all the tab items
- center - tabs items are aligned in the center
- centerSelected - tab items move to make the selected tab in the center
With this PR we have allowed styling of Tabs TabStrip in iOS. You can now change the font, background-color, color, and TextTransform. There are some limitations though:
- The iOS MDCTabBar supports only two styles of the tab strip items - normal and selected, which means that you cannot have different styles for two items with the same normal state
- The supported TextTransform values for the Tabs TabStrip are "none", "initial" and "uppercase"
- If the iOSTabBarItemsAlignment is set to something different than justify and you want to set the background of the selected item, you can achieve it by setting :active style to the TabStrip item, but the selection highlight indicator will not be shown in that case

Fixes #8369, #8345, #8340.

BREAKING CHANGES:

The default value of the TextTransform for android is now uppercase.

Migration steps:

If you don't want to use uppercased items, you need to change the TextTransform property/style either on TabStrip, TabStripItem or Label element.

vmutafov · 2020-02-21T16:07:21Z

+
+    if (!tabStripItem.iconSource) {
+        if (isIconAboveTitle) {
+            tabBarItem.titlePositionAdjustment = { horizontal: 0, vertical: -20 };


Where do these magic numbers come from?

I'm not sure. The same numbers are set in tabs-view and bottom-navigation as well. I haven't change it though.

vmutafov · 2020-02-21T16:11:53Z

+                return "initial";
+            case MDCTabBarTextTransform.Uppercase:
+            default:
+                return "uppercase";


Wouldn't it be better to throw an exception in the default branch? I think it would be easier to find out if for some reason there is a new property in the enumeration.

The thing is that this is something that is set through CSS and it works of Android for all the cases, but iOS doesn't support all, so I would prefer not to crash the app with a CSS change. On the other hand, if you cannot achieve what you want with a single style you can always use platform-specific ones.

vmutafov · 2020-03-06T13:47:16Z

+    private getItemLabelTextTransform(tabStripItem: TabStripItem): TextTransform {
+        const nestedLabel = tabStripItem.label;
+        let textTransform: TextTransform = null;
+        if (nestedLabel && nestedLabel.style.textTransform && nestedLabel.style.textTransform !== "initial") {


Shouldn't the check be if (nestedLabel && nestedLabel.style && nestedLabel.style.textTransform !== "initial")?
The same goes a couple of lines below.

I don't think this is needed: https://github.com/NativeScript/NativeScript/blob/master/nativescript-core/ui/core/view-base/view-base.ts#L259

Shouldn't it be if (nestedLabel && nestedLabel.style.textTransform !== "initial") then?

vmutafov · 2020-03-06T13:56:08Z

    }

+    get textTransform(): TextTransform {
+        return this.style.textTransform;


Is it possible we don't have a style?

I don't think so - https://github.com/NativeScript/NativeScript/blob/master/nativescript-core/ui/core/view-base/view-base.ts#L259

vmutafov · 2020-03-06T14:01:02Z

+    private getItemLabelTextTransform(tabStripItem: TabStripItem): TextTransform {
+        const nestedLabel = tabStripItem.label;
+        let textTransform: TextTransform = null;
+        if (nestedLabel && nestedLabel.style.textTransform && nestedLabel.style.textTransform !== "initial") {


Is the if check correct? Please check one of my previous comments. Also, isn't this duplicate code? Could we move it to a utility class for the TabStripItem?

It is duplicate, I've planned to create a base Tabs/Bottom Navigation component as there's a lot of duplicate code between the two controls.

Update images after changes caused by NativeScript/NativeScript#8366

* fix(tabs): delay loadView when animation runs * chore: update api.md * chore: remove unnecessary casting * test: Added disabled test for changing tabs * tabs(ios): added tabs styling in ios * tabs: added iosAlignment property * tabs: textTransform support * tabs: iosAlignment moved to tabstrip * test: add frame-in-tabs test * chore: addressing PR comments * chore: addressing PR comments * chore: call method on the instance instead of call * chore: move IOSAlignment property * chore: update comments * fix: texttransform to tabstrip in bottomnavigation * chore: add new item to native-api-usage * chore: remove unneeded setNativeView call * chore: removed unneeded check Co-authored-by: Dimitar Topuzov <dtopuzov@gmail.com>

vtrifonov added 9 commits February 18, 2020 10:13

fix(tabs): delay loadView when animation runs

ed784a0

chore: update api.md

ac86afa

chore: remove unnecessary casting

1c6d70a

test: Added disabled test for changing tabs

bac983b

tabs(ios): added tabs styling in ios

bb6fa93

tabs: added iosAlignment property

b27bf03

tabs: textTransform support

c7e3e0d

tabs: iosAlignment moved to tabstrip

226b7a5

test: add frame-in-tabs test

9f46c12

cla-bot Bot added the cla: yes label Feb 21, 2020

Merge branch 'master' into trifonov/tabs-styling

96fbf97

vmutafov reviewed Feb 21, 2020

View reviewed changes

vtrifonov added 4 commits February 24, 2020 14:18

chore: addressing PR comments

17467a9

Merge branch 'master' into trifonov/tabs-styling

b3f1d8d

chore: addressing PR comments

a6249d7

chore: call method on the instance instead of call

868b57e

vtrifonov changed the title ~~Trifonov/tabs styling~~ Tabs styling in iOS Feb 24, 2020

vtrifonov changed the title ~~Tabs styling in iOS~~ Tabs styling improvements Feb 24, 2020

vtrifonov added 2 commits February 24, 2020 16:16

chore: move IOSAlignment property

5d89c62

chore: update comments

b28e032

vtrifonov marked this pull request as ready for review February 24, 2020 14:27

vtrifonov and others added 5 commits February 24, 2020 16:59

fix: texttransform to tabstrip in bottomnavigation

2893fe9

Merge branch 'master' into trifonov/tabs-styling

3ed020b

Merge branch 'master' into trifonov/tabs-styling

0014af5

chore: add new item to native-api-usage

9ed4c21

chore: remove unneeded setNativeView call

dfe2792

vtrifonov requested a review from vmutafov March 5, 2020 17:24

Merge branch 'master' into trifonov/tabs-styling

b4b245d

vmutafov reviewed Mar 9, 2020

View reviewed changes

vmutafov approved these changes Mar 9, 2020

View reviewed changes

chore: removed unneeded check

11c76eb

dtopuzov added a commit to NativeScript/functional-tests-images that referenced this pull request Mar 9, 2020

chore: update images

cae8c4b

Update images after changes caused by NativeScript/NativeScript#8366

dtopuzov mentioned this pull request Mar 9, 2020

chore: update images NativeScript/functional-tests-images#54

Merged

vtrifonov merged commit 4589431 into master Mar 9, 2020

vtrifonov deleted the trifonov/tabs-styling branch March 9, 2020 13:24

Uh oh!

Conversation

vtrifonov commented Feb 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

What is the current behavior?

What is the new behavior?

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vtrifonov commented Feb 21, 2020 •

edited

Loading