-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Add indicators to let the user know if they are on staging or dev #2506
Changes from 10 commits
58edb14
2b5e6eb
c679039
432d29c
65a5ee5
f6608c6
00dfb7a
4c49990
f9ae00e
c08fd7f
7e12f38
0106b20
92c4a8d
e50a700
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
import React from 'react'; | ||
import {Text, View} from 'react-native'; | ||
import styles from '../styles/styles'; | ||
import borders from '../styles/utilities/borders'; | ||
import CONFIG from '../CONFIG'; | ||
import CONST from '../CONST'; | ||
|
||
const EnvironmentBadge = () => { | ||
// If we are on production, don't show any badge | ||
if (CONFIG.EXPENSIFY.ENVIRONMENT === CONST.ENVIRONMENT.PRODUCTION) { | ||
return null; | ||
} | ||
|
||
let badgeText = 'DEV'; | ||
Jag96 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
let style = styles.badgeDanger; | ||
Jag96 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
if (CONFIG.EXPENSIFY.ENVIRONMENT === CONST.ENVIRONMENT.STAGING) { | ||
badgeText = 'STG'; | ||
Jag96 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
style = styles.badgeSuccess; | ||
} | ||
|
||
return ( | ||
<View | ||
style={[styles.badge, borders.brSmall, style, styles.ml2]} | ||
> | ||
<Text | ||
style={styles.badgeText} | ||
numberOfLines={1} | ||
Jag96 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
> | ||
{badgeText} | ||
</Text> | ||
</View> | ||
); | ||
}; | ||
|
||
EnvironmentBadge.displayName = 'EnvironmentBadge'; | ||
export default EnvironmentBadge; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
import React from 'react'; | ||
import PropTypes from 'prop-types'; | ||
import ProductionLogo from '../../assets/images/expensify-cash.svg'; | ||
import DevLogo from '../../assets/images/expensify-cash-dev.svg'; | ||
import StagingLogo from '../../assets/images/expensify-cash-stg.svg'; | ||
import CONFIG from '../CONFIG'; | ||
import CONST from '../CONST'; | ||
|
||
const propTypes = { | ||
width: PropTypes.number.isRequired, | ||
height: PropTypes.number.isRequired, | ||
}; | ||
|
||
const ExpensifyCashLogo = (props) => { | ||
if (CONFIG.EXPENSIFY.ENVIRONMENT === CONST.ENVIRONMENT.PRODUCTION) { | ||
Jag96 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return <ProductionLogo width={props.width} height={props.height} />; | ||
} | ||
|
||
if (CONFIG.EXPENSIFY.ENVIRONMENT === CONST.ENVIRONMENT.STAGING) { | ||
return <StagingLogo width={props.width} height={props.height} />; | ||
} | ||
|
||
return <DevLogo width={props.width} height={props.height} />; | ||
}; | ||
|
||
ExpensifyCashLogo.propTypes = propTypes; | ||
ExpensifyCashLogo.displayName = 'ExpensifyCashLogo'; | ||
|
||
export default ExpensifyCashLogo; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -231,7 +231,6 @@ const styles = { | |
|
||
badge: { | ||
backgroundColor: themeColors.badgeDefaultBG, | ||
borderRadius: 14, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For the IOUbadge, this border-radius is now set to 12 instead of 14 since I wanted to keep the increments of small/medium/large as multiples of 4 like we do for the spacing file. @shawnborton let me know if this should be set back to 14 and I can update the value in the new borders.js file. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think for badges, we want them to look like rounded pills and not rounded rectangles because buttons look like rounded rectangles. So maybe go the other way and jack this up to 16 or 20? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm I am not seeing a difference. Just to make sure we're on the same page, I think the style we are using for the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah ok sounds good, I'll revert the styles then so both badges have the old border-radius that the IOUBadge had There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For some reason the pill doesn't look perfectly rounded, but maybe my eyes are playing tricks on me. Can you double check that the "DEV" pill is only 20px tall? if that's the case, a border radius of > 10px should make it perfectly round. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it just me, or is the text in the pill (both There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we see w/ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
height: variables.iconSizeNormal, | ||
flexDirection: 'row', | ||
paddingHorizontal: 7, | ||
|
@@ -242,6 +241,10 @@ const styles = { | |
backgroundColor: themeColors.badgeSuccessBG, | ||
}, | ||
|
||
badgeDanger: { | ||
backgroundColor: themeColors.badgeDangerBG, | ||
}, | ||
|
||
badgeText: { | ||
color: themeColors.textReversed, | ||
fontSize: variables.fontSizeSmall, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
import variables from '../variables'; | ||
|
||
/** | ||
* Spacing utility styles with Bootstrap inspired naming. | ||
* | ||
* https://getbootstrap.com/docs/5.0/utilities/borders | ||
*/ | ||
export default { | ||
brSmall: { | ||
borderRadius: variables.componentBorderRadiusSmall, | ||
}, | ||
|
||
brNormal: { | ||
borderRadius: variables.componentBorderRadiusNormal, | ||
}, | ||
|
||
brLarge: { | ||
borderRadius: variables.componentBorderRadiusLarge, | ||
}, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On dev,
Electron
is still shown in the dock and in the OS menu bar. https://www.electronjs.org/docs/tutorial/application-distribution had some examples on how to make this display properly on mac but I wasn't able to sort it out. Open to suggestions to fix that.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I personally think that this is fine to leave on dev.