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

fix inline code blocks #2527

Merged
merged 8 commits into from
Apr 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/components/AnchorForCommentsOnly/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from 'react';
import {StyleSheet} from 'react-native';
import {StyleSheet, Text} from 'react-native';
import anchorForCommentsOnlyPropTypes from './anchorForCommentsOnlyPropTypes';

const defaultProps = {
Expand All @@ -18,16 +18,17 @@ const AnchorForCommentsOnly = ({
style,
...props
}) => (
<a
<Text
style={StyleSheet.flatten(style)}
accessibilityRole="link"
href={href}
rel={rel}
target={target}
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
>
{children}
</a>
</Text>
);

AnchorForCommentsOnly.propTypes = anchorForCommentsOnlyPropTypes;
Expand Down
76 changes: 76 additions & 0 deletions src/components/InlineCodeBlock/WrappedText.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import _ from 'underscore';
import React, {Fragment} from 'react';
import {Text, View} from 'react-native';
import PropTypes from 'prop-types';
import styles from '../../styles/styles';

/**
* Breaks the text into matrix
* for eg: My Name is Rajat
* [
* [My,' ',Name,' ',' ',is,' ',Rajat],
* ]
*
* @param {String} text
* @returns {Array<String[]>}
*/
function getTextMatrix(text) {
return text.split('\n').map(row => _.without(row.split(/(\s)/), ''));
}
Comment on lines +17 to +19
Copy link
Contributor

Choose a reason for hiding this comment

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

This caused a regression #19922 when using emojis and text the inline code will look misaligned #19922 (comment).


const propTypes = {
// Required text
children: PropTypes.string.isRequired,

// Style to be applied to Text
textStyles: PropTypes.arrayOf(PropTypes.object),

// Style for each word(Token) in the text,
// remember that token also includes whitespaces among words
wordStyles: PropTypes.arrayOf(PropTypes.object),
};

const defaultProps = {
textStyles: [],
wordStyles: [],
};

const WrappedText = (props) => {
const textMatrix = getTextMatrix(props.children);
return (
<>
{textMatrix.map((rowText, rowIndex) => (
<Fragment
// eslint-disable-next-line react/no-array-index-key
key={`${rowText}-${rowIndex}`}
>
{rowText.map((colText, colIndex) => (

// Outer View is important to vertically center the Text
<View
// eslint-disable-next-line react/no-array-index-key
key={`${colText}-${colIndex}`}
style={styles.codeWordWrapper}
>
<View
style={[
props.wordStyles,
colIndex === 0 && styles.codeFirstWordStyle,
colIndex === rowText.length - 1 && styles.codeLastWordStyle,
]}
>
<Text style={props.textStyles}>{colText}</Text>
</View>
</View>
))}
</Fragment>
))}
</>
);
};

WrappedText.propTypes = propTypes;
WrappedText.defaultProps = defaultProps;
WrappedText.displayName = 'WrappedText';

export default WrappedText;
19 changes: 0 additions & 19 deletions src/components/InlineCodeBlock/index.android.js

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,23 +1,24 @@
/* eslint-disable react/jsx-props-no-spreading */
import React from 'react';
import {View} from 'react-native';
import styles from '../../styles/styles';
import WrappedText from './WrappedText';
import inlineCodeBlockPropTypes from './inlineCodeBlockPropTypes';

const InlineCodeBlock = ({
TDefaultRenderer,
defaultRendererProps,
boxModelStyle,
textStyle,
}) => (
<View
style={{
...boxModelStyle,
...styles.mbn1,
}}
<WrappedText
textStyles={[textStyle]}
wordStyles={[
boxModelStyle,
styles.codeWordStyle,
]}
// eslint-disable-next-line react/jsx-props-no-spreading
{...defaultRendererProps}
>
<TDefaultRenderer style={textStyle} {...defaultRendererProps} />
</View>
{defaultRendererProps.tnode.data}
</WrappedText>
);

InlineCodeBlock.propTypes = inlineCodeBlockPropTypes;
Expand Down
14 changes: 14 additions & 0 deletions src/styles/codeStyles/index.android.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
const codeWordWrapper = {
height: 10,
};

const codeWordStyle = {
top: -1,
marginVertical: -2,
};

const codeTextStyle = {
lineHeight: 18,
};

export default {codeWordWrapper, codeWordStyle, codeTextStyle};
16 changes: 16 additions & 0 deletions src/styles/codeStyles/index.ios.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
const codeWordWrapper = {
height: 18,
justifyContent: 'center',
marginTop: 2,
};

const codeWordStyle = {
height: 16,
top: 4,
};

const codeTextStyle = {
lineHeight: 15,
};
Comment on lines +12 to +14
Copy link
Contributor

Choose a reason for hiding this comment

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

The line height here was not enough and caused a regression #19922 when the inline code contains emojis.


export default {codeWordWrapper, codeWordStyle, codeTextStyle};
5 changes: 5 additions & 0 deletions src/styles/codeStyles/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// We do not need these on Web/Desktop as their implementation defer from Native devices so just noop them
const codeWordWrapper = {};
const codeWordStyle = {};
const codeTextStyle = {};
export default {codeWordWrapper, codeWordStyle, codeTextStyle};
39 changes: 35 additions & 4 deletions src/styles/styles.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import wordBreak from './utilities/wordBreak';
import textInputAlignSelf from './utilities/textInputAlignSelf';
import CONST from '../CONST';
import positioning from './utilities/positioning';
import codeStyles from './codeStyles';

const styles = {
// Add all of our utility and helper styles
Expand Down Expand Up @@ -1312,6 +1313,37 @@ const styles = {
scrollbarWidth: 'none',
},

codeWordWrapper: {
...codeStyles.codeWordWrapper,
},

codeWordStyle: {
borderLeftWidth: 0,
borderRightWidth: 0,
borderTopLeftRadius: 0,
borderBottomLeftRadius: 0,
borderTopRightRadius: 0,
borderBottomRightRadius: 0,
paddingLeft: 0,
paddingRight: 0,
justifyContent: 'center',
...codeStyles.codeWordStyle,
},

codeFirstWordStyle: {
borderLeftWidth: 1,
borderTopLeftRadius: 4,
borderBottomLeftRadius: 4,
paddingLeft: 5,
},

codeLastWordStyle: {
borderRightWidth: 1,
borderTopRightRadius: 4,
borderBottomRightRadius: 4,
paddingRight: 5,
},

fullScreenLoading: {
backgroundColor: themeColors.componentBG,
opacity: 0.8,
Expand All @@ -1332,8 +1364,6 @@ const styles = {
const baseCodeTagStyles = {
borderWidth: 1,
borderRadius: 5,
marginTop: 4,
marginBottom: 4,
borderColor: themeColors.border,
backgroundColor: themeColors.textBackground,
};
Expand Down Expand Up @@ -1388,11 +1418,11 @@ const webViewStyles = {

code: {
...baseCodeTagStyles,
...codeStyles.codeTextStyle,
paddingLeft: 5,
paddingRight: 5,
paddingBottom: 2,
alignSelf: 'flex-start',
fontFamily: fontFamily.MONOSPACE,
fontSize: 13,
},

img: {
Expand All @@ -1405,6 +1435,7 @@ const webViewStyles = {
baseFontStyle: {
color: themeColors.text,
fontSize: variables.fontSizeNormal,
lineHeight: variables.fontSizeNormalHeight,
fontFamily: fontFamily.GTA,
},
};
Expand Down
1 change: 1 addition & 0 deletions src/styles/variables.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export default {
fontSizeLarge: 17,
fontSizeh1: 19,
iconSizeExtraSmall: 12,
fontSizeNormalHeight: 20,
iconSizeSmall: 16,
iconSizeNormal: 20,
iconSizeLarge: 24,
Expand Down