Skip to content

Commit

Permalink
move Hermes badge from template to NewAppScreen library (#28783)
Browse files Browse the repository at this point in the history
Summary:
Refs #28711

This PR moves Hermes badge component from the template to the `NewAppScreen` library. The main motivation behind this change was to simplify a bit template code.

I assumed that it is not important to expose `global.HermesInternal` to the template users:
* If this assumption is true, I think that there are no other reason to leave this component inside `App` in comparison to other `NewAppScreen` components,
* If this assumption is false, I can adjust this PR and move `HermesInternal` check from the badge component to the `App`.

I was trying to avoid calling `useColorScheme` when Hermes is disabled, but placing hook inside the conditional branch causes ESLint warning (react-hooks/rules-of-hooks).

This PR includes also small style tweaks for the badge - since there are no background padding can be omitted and spacing can be added adjusted tweaking `top` and `left` properties and `fontSize` has been adjusted just for the readability.

In the last commit, I have gone a bit further and moved `HermesBadge` to the `Header` component and I have also changed slightly the `Header` title (React -> React Native) and it's styling.
> I'm not sure if after this change `HermesBadge` export in `NewAppScreen` components list is still required, but maybe this badge will be useful for someone. If it's a mistake I can update the PR and remove this export.

## Changelog

[Internal][Changed] move Hermes badge from the template to the NewAppScreen library

Pull Request resolved: #28783

Test Plan:
Template app do not redbox on Android emulator with and without Hermes enabled.

## Preview
Android with Hermes enabled and adjusted header:

![Screenshot_1588164908](https://user-images.githubusercontent.com/719641/80599357-16dc8900-8a2b-11ea-8b3e-9a2cb26d3470.png)

iOS with adjusted header:

![IMG_6551](https://user-images.githubusercontent.com/719641/80599445-3bd0fc00-8a2b-11ea-8215-318625ddad13.PNG)

Reviewed By: GijsWeterings

Differential Revision: D22493822

Pulled By: cpojer

fbshipit-source-id: 3440e10f2d59f268ca8851a6e002f0ff23fa839c
  • Loading branch information
Simek authored and facebook-github-bot committed Jul 14, 2020
1 parent 4a97791 commit 1c634a9
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 31 deletions.
10 changes: 7 additions & 3 deletions Libraries/NewAppScreen/components/Header.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@
*/

'use strict';
import Colors from './Colors';
import type {Node} from 'react';
import {ImageBackground, StyleSheet, Text, useColorScheme} from 'react-native';
import React from 'react';
import Colors from './Colors';
import HermesBadge from './HermesBadge';

const Header = (): Node => {
const isDarkMode = useColorScheme() === 'dark';
Expand All @@ -27,14 +28,17 @@ const Header = (): Node => {
},
]}
imageStyle={styles.logo}>
<HermesBadge />
<Text
style={[
styles.text,
{
color: isDarkMode ? Colors.white : Colors.black,
},
]}>
Welcome to React
Welcome to
{'\n'}
React Native
</Text>
</ImageBackground>
);
Expand All @@ -61,7 +65,7 @@ const styles = StyleSheet.create({
},
text: {
fontSize: 40,
fontWeight: '600',
fontWeight: '700',
textAlign: 'center',
},
});
Expand Down
46 changes: 46 additions & 0 deletions Libraries/NewAppScreen/components/HermesBadge.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow strict-local
* @format
*/

import React from 'react';
import type {Node} from 'react';
import {StyleSheet, Text, useColorScheme, View} from 'react-native';
import Colors from './Colors';

const HermesBadge = (): Node => {
const isDarkMode = useColorScheme() === 'dark';
return global.HermesInternal ? (
<View style={styles.badge}>
<Text
style={[
styles.badgeText,
{
color: isDarkMode ? Colors.light : Colors.dark,
},
]}>
Engine: Hermes
</Text>
</View>
) : null;
};

const styles = StyleSheet.create({
badge: {
position: 'absolute',
top: 8,
right: 12,
},
badgeText: {
fontSize: 14,
fontWeight: '600',
textAlign: 'right',
},
});

export default HermesBadge;
12 changes: 10 additions & 2 deletions Libraries/NewAppScreen/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,18 @@

'use strict';

import Colors from './components/Colors';
import Header from './components/Header';
import HermesBadge from './components/HermesBadge';
import LearnMoreLinks from './components/LearnMoreLinks';
import Colors from './components/Colors';
import DebugInstructions from './components/DebugInstructions';
import ReloadInstructions from './components/ReloadInstructions';

export {Header, LearnMoreLinks, Colors, DebugInstructions, ReloadInstructions};
export {
Colors,
Header,
HermesBadge,
LearnMoreLinks,
DebugInstructions,
ReloadInstructions,
};
26 changes: 0 additions & 26 deletions template/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,28 +59,13 @@ const App: () => Node = () => {
backgroundColor: isDarkMode ? Colors.darker : Colors.lighter,
};

const hermes = global.HermesInternal ? (
<View style={styles.engine}>
<Text
style={[
styles.footer,
{
color: isDarkMode ? Colors.light : Colors.dark,
},
]}>
Engine: Hermes
</Text>
</View>
) : null;

return (
<SafeAreaView style={backgroundStyle}>
<StatusBar barStyle={isDarkMode ? 'light-content' : 'dark-content'} />
<ScrollView
contentInsetAdjustmentBehavior="automatic"
style={backgroundStyle}>
<Header />
{hermes}
<View
style={{
backgroundColor: isDarkMode ? Colors.black : Colors.white,
Expand All @@ -106,10 +91,6 @@ const App: () => Node = () => {
};

const styles = StyleSheet.create({
engine: {
position: 'absolute',
right: 0,
},
sectionContainer: {
marginTop: 32,
paddingHorizontal: 24,
Expand All @@ -126,13 +107,6 @@ const styles = StyleSheet.create({
highlight: {
fontWeight: '700',
},
footer: {
fontSize: 12,
fontWeight: '600',
padding: 4,
paddingRight: 12,
textAlign: 'right',
},
});

export default App;

0 comments on commit 1c634a9

Please sign in to comment.