Skip to content

Commit 92c9b0f

Browse files
chrisbobbegnprice
authored andcommitted
ZulipText [nfc]: Offer inherit{FontSize,Color} props
This is a new approach to dealing with ZulipText's aggressiveDefaultStyles. Like the current one, it has benefits and tradeoffs. Here's an example of an upside: in RealmInputScreen, we can now group three ZulipText(Intl)s together under one ZulipText that provides `styles.suggestionText` through inheritance, without having to repeat the very specific `fontSize: 12` in some form across all three of them. We do have to pass a new prop, inheritFontSize, but that actually feels appropriate here: those three text components are overriding a global default, after all, and now they all have breadcrumbs leading back to the one element that sets the overriding value. Oh, and another nice thing: WebLink's implementation is much less finicky now! I'm not aware of any bugs it has caused, but it feels easier to understand, and more resilient to upgrading React across breaking changes. A downside is that WebLink's children now have to know that WebLink's set of provided styles includes a `color` and do a little incantation (`inheritColor`) to let that work. It seems natural for WebLink's callers to be aware of that, but the incantation could get tiresome if WebLink (or some analogous component) wanted to apply a bunch of other attributes, like a text underline, etc.
1 parent 72280dd commit 92c9b0f

File tree

4 files changed

+36
-48
lines changed

4 files changed

+36
-48
lines changed

src/common/WebLink.js

Lines changed: 3 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
import * as React from 'react';
44

55
import ZulipText from './ZulipText';
6-
import ZulipTextIntl from './ZulipTextIntl';
76
import { openLinkWithUserPreference } from '../utils/openLink';
87
import { BRAND_COLOR, createStyleSheet } from '../styles';
98
import { useGlobalSelector } from '../react-redux';
@@ -25,16 +24,8 @@ const componentStyles = createStyleSheet({
2524
*
2625
* Accepts strings as children or a `text` prop, just like ZulipText.
2726
*
28-
* Also accepts `ZulipText`s and `ZulipTextIntl`s as children.
29-
*
30-
* Note: This sort of messes in those non-string children's business by
31-
* unsetting some of their default style attributes. It does this so that
32-
* its own special formatting can be passed down through the limited style
33-
* inheritance that RN supports:
34-
* https://reactnative.dev/docs/text#limited-style-inheritance
35-
* See `aggressiveDefaultStyle` in ZulipText.
36-
*
37-
* TODO: Possibly there's a better way handle this.
27+
* Also accepts `ZulipText`s and `ZulipTextIntl`s as children. To keep the
28+
* special formatting, you have to pass `inheritColor` to those.
3829
*/
3930
export default function WebLink(props: Props): React.Node {
4031
const { children } = props;
@@ -48,30 +39,7 @@ export default function WebLink(props: Props): React.Node {
4839
openLinkWithUserPreference(props.url.toString(), globalSettings);
4940
}}
5041
>
51-
{React.Children.map(children, child => {
52-
if (!React.isValidElement(child)) {
53-
// Some React node that isn't a React element (a `React.Element`);
54-
// e.g., a plain string. The enclosing ZulipText will apply its
55-
// styles directly.
56-
return child;
57-
}
58-
// `child` should be a React.Element at this point. Docs are very
59-
// vague, but this sounds like it should be true, and it seems true
60-
// empirically. Would at least be good to have better type checking.
61-
62-
if (child.type !== ZulipText && child.type !== ZulipTextIntl) {
63-
return child;
64-
}
65-
// These element types will have a style prop that we want to add to.
66-
67-
return React.cloneElement(child, {
68-
style: {
69-
// Defeat ZulipText's aggressiveDefaultStyle.color so that our
70-
// color gets applied.
71-
color: undefined,
72-
},
73-
});
74-
})}
42+
{children}
7543
</ZulipText>
7644
);
7745
}

src/common/ZulipText.js

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,16 @@ import invariant from 'invariant';
33
import React, { useContext } from 'react';
44
import type { Node } from 'react';
55
import { Text } from 'react-native';
6+
import type { ____TextStyle_Internal } from 'react-native/Libraries/StyleSheet/StyleSheetTypes'; // eslint-disable-line id-match
67

78
import { ThemeContext } from '../styles';
89

910
type Props = $ReadOnly<{|
1011
...$Exact<React$ElementConfig<typeof Text>>,
1112
text?: string,
13+
14+
inheritFontSize?: boolean,
15+
inheritColor?: boolean,
1216
|}>;
1317

1418
/**
@@ -20,11 +24,21 @@ type Props = $ReadOnly<{|
2024
*
2125
* @prop text - Contents for Text.
2226
* @prop [style] - Can override our default style for this component.
27+
* @prop inheritFontSize, etc. - Use instead of `styles.fontSize`, etc., to
28+
* inherit value from an ancestor Text in the layout:
29+
* https://reactnative.dev/docs/0.67/text#limited-style-inheritance
2330
* @prop ...all other Text props - Passed through verbatim to Text.
2431
* See upstream: https://reactnative.dev/docs/text
2532
*/
2633
export default function ZulipText(props: Props): Node {
27-
const { text, children, style, ...restProps } = props;
34+
const {
35+
text,
36+
children,
37+
style,
38+
inheritFontSize = false,
39+
inheritColor = false,
40+
...restProps
41+
} = props;
2842
const themeData = useContext(ThemeContext);
2943

3044
invariant(
@@ -33,14 +47,22 @@ export default function ZulipText(props: Props): Node {
3347
);
3448
invariant(text == null || children == null, 'ZulipText: `text` or `children` should be nullish');
3549

36-
// These attributes will be applied unless specifically overridden
37-
// with the `style` prop -- even if this `<ZulipText />` is nested
38-
// and would otherwise inherit the attributes from its ancestors.
39-
const aggressiveDefaultStyle = {
50+
// These attributes will be applied unless specifically overridden with
51+
// the `style` or `inherit*` props -- even if this `<ZulipText />` is
52+
// nested and would otherwise inherit the attributes from its ancestors.
53+
const aggressiveDefaultStyle: $Rest<____TextStyle_Internal, { ... }> = {
4054
fontSize: 15,
4155
color: themeData.color,
56+
// If adding an attribute `foo`, give callers a prop `inheritFoo`.
4257
};
4358

59+
if (inheritFontSize) {
60+
delete aggressiveDefaultStyle.fontSize;
61+
}
62+
if (inheritColor) {
63+
delete aggressiveDefaultStyle.color;
64+
}
65+
4466
return (
4567
<Text style={[aggressiveDefaultStyle, style]} {...restProps}>
4668
{text}

src/start/PasswordAuthScreen.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ class PasswordAuthScreenInner extends PureComponent<Props, State> {
144144
<View style={styles.linksTouchable}>
145145
<ZulipText style={styles.forgotPasswordText}>
146146
<WebLink url={new URL('/accounts/password/reset/', realm)}>
147-
<ZulipTextIntl text="Forgot password?" />
147+
<ZulipTextIntl inheritColor text="Forgot password?" />
148148
</WebLink>
149149
</ZulipText>
150150
</View>

src/start/RealmInputScreen.js

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ export default function RealmInputScreen(props: Props): Node {
221221
[themeContext],
222222
);
223223

224-
const renderedSuggestion = React.useMemo(() => {
224+
const suggestionContent = React.useMemo(() => {
225225
if (suggestion === null) {
226226
// Vertical spacer so the layout doesn't jump when a suggestion
227227
// appears or disappears. (The empty string might be neater, but it
@@ -230,13 +230,11 @@ export default function RealmInputScreen(props: Props): Node {
230230
// and React or RN gave in to that. I've tried the obvious ways to use
231231
// RN's PixelRatio.getFontScale() and never got the right height
232232
// either; dunno why.)
233-
return (
234-
<ZulipText style={styles.suggestionText} text={'\u200b'} /* U+200B ZERO WIDTH SPACE */ />
235-
);
233+
return '\u200b'; // U+200B ZERO WIDTH SPACE
236234
} else if (typeof suggestion === 'string') {
237235
return (
238236
<ZulipTextIntl
239-
style={styles.suggestionText}
237+
inheritFontSize
240238
text={{
241239
text: 'Suggestion: <z-link>{suggestedServerUrl}</z-link>',
242240
values: {
@@ -254,7 +252,7 @@ export default function RealmInputScreen(props: Props): Node {
254252
/>
255253
);
256254
} else {
257-
return <ZulipTextIntl style={styles.suggestionText} text={validationErrorMsg(suggestion)} />;
255+
return <ZulipTextIntl inheritFontSize text={validationErrorMsg(suggestion)} />;
258256
}
259257
}, [suggestion, handlePressSuggestion, styles]);
260258

@@ -299,7 +297,7 @@ export default function RealmInputScreen(props: Props): Node {
299297
ref={textInputRef}
300298
/>
301299
</View>
302-
{renderedSuggestion}
300+
<ZulipText style={styles.suggestionText}>{suggestionContent}</ZulipText>
303301
<ZulipButton
304302
style={styles.button}
305303
text="Enter"

0 commit comments

Comments
 (0)