Skip to content
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
3 changes: 2 additions & 1 deletion src/components/AnchorForAttachmentsOnly/index.native.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import React from 'react';
import * as anchorForAttachmentsOnlyPropTypes from './anchorForAttachmentsOnlyPropTypes';
import BaseAnchorForAttachmentsOnly from './BaseAnchorForAttachmentsOnly';
import * as StyleUtils from '../../styles/StyleUtils';
import styles from '../../styles/styles';

// eslint-disable-next-line react/jsx-props-no-spreading
const AnchorForAttachmentsOnly = props => <BaseAnchorForAttachmentsOnly {...props} style={styles.mw100} />;
const AnchorForAttachmentsOnly = props => <BaseAnchorForAttachmentsOnly {...props} style={[...StyleUtils.parseStyleAsArray(props.style), styles.mw100]} />;

AnchorForAttachmentsOnly.propTypes = anchorForAttachmentsOnlyPropTypes.propTypes;
AnchorForAttachmentsOnly.defaultProps = anchorForAttachmentsOnlyPropTypes.defaultProps;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import htmlRenderers from './HTMLRenderers';
import * as HTMLEngineUtils from './htmlEngineUtils';
import styles from '../../styles/styles';
import fontFamily from '../../styles/fontFamily';
import defaultViewProps from './defaultViewProps';

const propTypes = {
/** Whether text elements should be selectable */
Expand Down Expand Up @@ -50,8 +51,6 @@ const customHTMLElementModels = {
}),
};

const defaultViewProps = {style: [styles.alignItemsStart, styles.userSelectText]};

// We are using the explicit composite architecture for performance gains.
// Configuration for RenderHTML is handled in a top-level component providing
// context to RenderHTMLSource components. See https://git.io/JRcZb
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ const AnchorRenderer = (props) => {
if (isAttachment) {
return (
<AnchorForAttachmentsOnly
style={styles.alignItemsStart}
source={tryResolveUrlFromApiRoot(attrHref)}
displayName={displayName}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ const ImageRenderer = (props) => {
>
{({show}) => (
<PressableWithoutFocus
style={styles.noOutline}
styles={[styles.noOutline, styles.alignItemsStart]}
onPress={show}
onLongPress={event => showContextMenuForReport(event, anchor, reportID, action, checkIfContextMenuActive)}
>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import styles from '../../../styles/styles';

export default {
style: [styles.dFlex, styles.userSelectText],

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hey, @aswin-s Do you mind explaining to me why we had to remove the styles.alignItemsStart from Native platforms?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

alignItemsStart was applied to the root renderer for both web and native platforms. While fixing it for web platform I removed it for both platforms and moved it to inner renderers which are responsible for rendering individual components. You can checkout the changes for Image and Attachment renders. This style is needed for both native and web platforms but applying it at root node has unintended effects for web.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So the change was needed only for web platforms right, and switch back only for native should make no harm? I came across the problem with dFlex on native, as with it applied on root render component, rendered text component span on whole line, and applying it to the renderer itself does nothing as it is intended to be inline rather than block

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it makes sense to keep the same styling layout across platforms as much as possible. This will reduce the complexity. For now, we have a simple solution that works so let's go with that.

};
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import styles from '../../../styles/styles';

export default {
// For web platform default to block display. Using flex on root view will force all
// child elements to be block elements even when they have display inline added to them.
// This will affect elements like <a> which are inline by default.
style: [styles.dBlock, styles.userSelectText],
Comment thread
aswin-s marked this conversation as resolved.
};

3 changes: 3 additions & 0 deletions src/styles/utilities/display.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,7 @@ export default {
dInline: {
display: 'inline',
},
dBlock: {
display: 'block',
},
};