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

TabBarIOS tintColor #1337

Closed
wants to merge 8 commits into from
Closed

TabBarIOS tintColor #1337

wants to merge 8 commits into from

Conversation

tsunammis
Copy link
Contributor

Origin Pull request from cmcewen

All the work have been done by @cmcewen, I just rebased his work with the master.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 19, 2015
@ericvicenti
Copy link
Contributor

Lucky number 1337!

Can you expand upon the tab bar example code to show this new functionality?

@ericvicenti ericvicenti changed the title Tabbar TabBarIOS tintColor May 19, 2015
@tsunammis
Copy link
Contributor Author

@ericvicenti do you know how launch UIExplorer e2e test directly on my desktop ?

I tried some commands

cd Examples/UIExplorer
xctool -scheme UIExplorer -sdk iphonesimulator8.1
./scripts/e2e-test.sh
./runXcodeTests.sh

But I can't reproduce the failing test

I have to retrieve the new screenshot and put it on the xamples/UIExplorer/UIExplorerTests/ReferenceImages/Examples-UIExplorer-UIExplorerApp/ folder, but I can't launch the test.

Anyone knows ?

@ericvicenti
Copy link
Contributor

Ah, so the snapshot tests can be finicky. Things will look different based on the device in the simulator, 32 vs 64 bit, OS version, etc. Are you re-recording the snapshot on iPhone 5 (32-bit) and iOS 8.x?

You should be able to run the tests locally with ./scripts/objc-test.sh

@tsunammis
Copy link
Contributor Author

Are you re-recording the snapshot on iPhone 5 (32-bit) and iOS 8.x?

Yeah, I record the snapshot with iOS 8.1 and iPhone 5 but I can't specify 32-bit.

The difference between my screenshot and the oldest, its the text size, mine is more smaller than the oldest. @ericvicenti Do you know why ?

@tsunammis
Copy link
Contributor Author

@ericvicenti it's ok ?

I changed the “recordMode” from NO to YES inside the UIExplorerTests, the “reference image” has been updated, and I can’t see my changes on the screenshot, but I see the changes when I run the UIExplorer project on Xcode, do you know why ? And by now, the tests pass but it is bullshit according to me.

What is the next steps to merge this feature ?

@tsunammis
Copy link
Contributor Author

@nicklockwood Do you have something to add ?

@nicklockwood
Copy link
Contributor

Running tests on iPhone 5 automatically makes them 32-bit, iPhone 5s and above is 64-bit.

I don't know why you wouldn't be seeing the changes in the reference image. That's very strange.

@@ -89,6 +91,9 @@ var TabBarExample = React.createClass({
});

var styles = StyleSheet.create({
tabBar: {
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this?

@tsunammis
Copy link
Contributor Author

@nicklockwood code updated ;-)

Can you try this branch if you see the tab bar in blue on the UIExplorer project ?

@tsunammis
Copy link
Contributor Author

@a2 @nicklockwood @ericvicenti merging is possible ?

@johanneslumpe
Copy link
Contributor

This would be nice to get merged! :)

@brentvatne
Copy link
Collaborator

Looks good to me

@tsunammis
Copy link
Contributor Author

@brentvatne what is the next step ? This could be merged friday during the time afforded to those things ? -> https://gist.github.com/brentvatne/e1ca1e4beaf0135e3178

@nicklockwood
Copy link
Contributor

This was actually merged yesterday. Should be included in the next update. Sorry for the wait!

@brentvatne
Copy link
Collaborator

👍 thanks @nicklockwood!

@brentvatne
Copy link
Collaborator

@tsunammis - looks like that won't be needed 😄 hope you join in on Friday!

@tsunammis
Copy link
Contributor Author

@brentvatne thank you !! I hope I will be available friday, what is the timezone ? I'm in Paris.
@nicklockwood thank you, you are the best !!

tadeuzagallo pushed a commit to tadeuzagallo/react-native that referenced this pull request May 28, 2015
Summary:
[Origin Pull request](facebook#961) from [cmcewen](https://github.com/cmcewen)

All the work have been done by @cmcewen, I just rebased his work with the master.
Closes facebook#1337
Github Author: Stan Chollet <stanislas.chollet@gmail.com>

Test Plan: Imported from GitHub, without a `Test Plan:` line.
@brentvatne
Copy link
Collaborator

@tsunammis - I'll be around at 10am - 3pm PT 😄

ayushjainrksh pushed a commit to MLH-Fellowship/react-native that referenced this pull request Jul 2, 2020
* Add linting for insensitive and inconsiderate language

* Add .alexrc.js

* Update alex rules

* Minor updates to formatting

* Fix deploy_website job name

* Update alex rules

* Add lintv and .alexignore

* Fix lint issues
ryanlntn pushed a commit to ryanlntn/react-native that referenced this pull request Aug 9, 2022
* Delete more stuff

* Remove some stale looking SDX platform scripts

* Remove remnant of `acceptsKeyboardFocus`

* Add back some missing newlines
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants