Page MenuHomePhabricator
Differential D13084

[native] Create muted tab tip
ClosedPublic
Actions

Authored by inka on Aug 14 2024, 7:06 AM.
Tags
None
Referenced Files
F2754490: D13084.id43391.diff
Wed, Sep 18, 7:46 PM
Unknown Object (File)
Wed, Sep 18, 1:14 PM
Unknown Object (File)
Wed, Sep 18, 1:12 AM
Unknown Object (File)
Tue, Sep 17, 7:23 PM
Unknown Object (File)
Tue, Sep 17, 7:21 PM
Unknown Object (File)
Tue, Sep 17, 2:56 PM
Unknown Object (File)
Tue, Sep 17, 2:38 PM
Unknown Object (File)
Tue, Sep 17, 12:05 PM
View All 88 Files
Subscribers
ashoat

Details

Reviewers
tomek
will
ashoat
Commits
rCOMM0a5a8544b159: [native] Create muted tab tip
Summary

issue: ENG-8544

Test Plan

Please see the video:

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

inka created this revision.Aug 14 2024, 7:06 AM
Herald added a subscriber: ashoat. · View Herald TranscriptAug 14 2024, 7:06 AM
inka added a parent revision: D13082: [native] Create custom top tabs component.Aug 14 2024, 7:06 AM
Harbormaster returned this revision to the author for changes because remote builds failed.Aug 14 2024, 7:10 AM
Harbormaster failed remote builds in B31102: Diff 43388!
inka edited the test plan for this revision. (Show Details)Aug 14 2024, 7:13 AM
inka attached a referenced file: F2472068: Simulator Screen Recording - iPhone 15 Pro - 2024-08-14 at 16.12.37.mp4. (Show Details)
inka updated this revision to Diff 43390.Aug 14 2024, 7:47 AM

Fix text

Harbormaster returned this revision to the author for changes because remote builds failed.Aug 14 2024, 7:52 AM
Harbormaster failed remote builds in B31104: Diff 43390!
inka updated this revision to Diff 43391.Aug 14 2024, 8:00 AM

Remove console.log

inka added a child revision: D13085: [native] Add nux handler.Aug 14 2024, 8:04 AM
Harbormaster completed remote builds in B31105: Diff 43391.Aug 14 2024, 8:18 AM
inka requested review of this revision.Aug 14 2024, 8:18 AM
tomek accepted this revision.Aug 14 2024, 10:05 AM
tomek added a reviewer: ashoat.
tomek added inline comments.
native/navigation/muted-tab-tip.react.js
27–32 ↗(On Diff #43391)

Memoizing simple strings usually isn't beneficial, but also shouldn't hurt.

inka added inline comments.Aug 20 2024, 12:49 AM
native/navigation/muted-tab-tip.react.js
27–32 ↗(On Diff #43391)

And I think it looks much better when the string is a result of a function call basically.
const str = if(condition){return 5;}else{return 10;} is much better than
let str = 10; if(condition){str=5;}.

tomek added inline comments.Aug 20 2024, 1:13 AM
native/navigation/muted-tab-tip.react.js
34–38 ↗(On Diff #43391)

It is a component that has the type ComponentType.

inka updated this revision to Diff 43475.Aug 20 2024, 1:52 AM

Rename

Harbormaster completed remote builds in B31159: Diff 43475.Aug 20 2024, 2:36 AM
ashoat requested changes to this revision.Aug 20 2024, 12:45 PM

The video in the Test Plan doesn't seem to show the right text. Can you share a screenshot of just mutedTabTipTextBase, and both mutedTabTipTextBase + mutedTabTipTextIfFID?

This revision now requires changes to proceed.Aug 20 2024, 12:45 PM
inka added a comment.Wed, Aug 21, 12:19 AM

Here are screenshots with updated styles according to what we discussed in ENG-9058

image.png (2×1 px, 318 KB)

image.png (2×1 px, 327 KB)

ashoat accepted this revision.Wed, Aug 21, 12:25 AM

Thanks!

This revision is now accepted and ready to land.Wed, Aug 21, 12:25 AM
inka updated this revision to Diff 43566.Thu, Aug 22, 2:46 AM

Add MutedTabTipRouteName to newReanimatedRoutes

Harbormaster failed remote builds in B31210: Diff 43566!Thu, Aug 22, 2:48 AM
inka updated this revision to Diff 43679.Tue, Aug 27, 6:02 AM

Update approach due to changes requested in D13082.

Harbormaster completed remote builds in B31287: Diff 43679.Tue, Aug 27, 6:19 AM
inka requested review of this revision.Tue, Aug 27, 6:31 AM

Requesting review because the changes are significant

I don't see a better way of doing this. The styles in materiar-top-tabs lib are inlined in MaterialTopTabBar.

inka added a comment.Tue, Aug 27, 6:35 AM

New video:

(light mode drawer tip looks a bit broken, fixed in D13176

inka attached a referenced file: F2556800: Simulator Screen Recording - iPhone 15 Pro Max - 2024-08-27 at 15.34.28.mp4. (Show Details)Tue, Aug 27, 6:36 AM
tomek accepted this revision.Tue, Aug 27, 7:38 AM
tomek added inline comments.
native/navigation/muted-tab-tip.react.js
22 ↗(On Diff #43679)

Is it intentional to have a leading white space?

This revision is now accepted and ready to land.Tue, Aug 27, 7:38 AM
inka added inline comments.Wed, Aug 28, 4:11 AM
native/navigation/muted-tab-tip.react.js
22 ↗(On Diff #43679)

Yes! We don't want a trailing white space in the previous string, since it might be displayed alone. But this string is always concatenated with the first. So it includes the white space that separates the sentences

inka updated this revision to Diff 43731.EditedWed, Aug 28, 4:11 AM

Rebase

Harbormaster completed remote builds in B31323: Diff 43731.Wed, Aug 28, 4:28 AM
Closed by commit rCOMM0a5a8544b159: [native] Create muted tab tip (authored by inka). · Explain WhyThu, Aug 29, 3:18 AM
This revision was automatically updated to reflect the committed changes.
inka added a commit: rCOMM0a5a8544b159: [native] Create muted tab tip.

Revision Contents
Changeset List

PathSize
native/
components/
3 lines
navigation/
7 lines
116 lines
6 lines
6 lines

Diff 43794

View Options

native/components/nux-tips-context.react.js

Loading...
View Options

native/navigation/app-navigator.react.js

Loading...
View Options

native/navigation/muted-tab-tip.react.js

Loading...
View Options

native/navigation/overlay-navigator.react.js

Loading...
View Options

native/navigation/route-names.js

Loading...