-
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
[No QA] create UserMentionRenderer for rendering mentions inside chat #18495
Changes from all commits
0133691
357bd29
d362ae2
76ec3a0
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,59 @@ | ||
import React from 'react'; | ||
import _ from 'underscore'; | ||
import { | ||
TNodeChildrenRenderer, | ||
} from 'react-native-render-html'; | ||
import Navigation from '../../../libs/Navigation/Navigation'; | ||
import ROUTES from '../../../ROUTES'; | ||
import Text from '../../Text'; | ||
import Tooltip from '../../Tooltip'; | ||
import htmlRendererPropTypes from './htmlRendererPropTypes'; | ||
import withCurrentUserPersonalDetails from '../../withCurrentUserPersonalDetails'; | ||
import personalDetailsPropType from '../../../pages/personalDetailsPropType'; | ||
import * as StyleUtils from '../../../styles/StyleUtils'; | ||
|
||
const propTypes = { | ||
...htmlRendererPropTypes, | ||
|
||
/** | ||
* Current user personal details | ||
*/ | ||
currentUserPersonalDetails: personalDetailsPropType.isRequired, | ||
}; | ||
|
||
/** | ||
* Navigates to user details screen based on email | ||
* @param {String} email | ||
* @returns {void} | ||
* */ | ||
const showUserDetails = email => Navigation.navigate(ROUTES.getDetailsRoute(email)); | ||
|
||
const MentionUserRenderer = (props) => { | ||
const defaultRendererProps = _.omit(props, ['TDefaultRenderer', 'style']); | ||
|
||
// We need to remove the leading @ from data as it is not part of the login | ||
const loginWhithoutLeadingAt = props.tnode.data.slice(1); | ||
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. Coming from #23811: More details about root cause: #23811 (comment) Repro step: #23811 (comment) |
||
|
||
const isOurMention = loginWhithoutLeadingAt === props.currentUserPersonalDetails.login; | ||
|
||
return ( | ||
<Text> | ||
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 this Wrapper 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. It wasn't like that in initial PR. So I traced why it is looking like this after rebasing this branch. After bisecting for a while I found this PR and it changes the defaultViewProps for native platforms from 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. So, you are saying that we should remove Dflex from native platforms. 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. I'd love to try and get this merged today if we can as we have a pretty quick timeline we're working with. So if we don't get a response soon, let's go forward with what we have. 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. Yes, I will get this merged today. 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. Sounds like we decided to keep it as-is? 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. looks like it, I'm here in case something would have to be changed 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. Yeah, let me test it and finish up the checklist. |
||
<Tooltip absolute text={loginWhithoutLeadingAt}> | ||
<Text | ||
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. 👋 Coming from #19303 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. Ahhhh good to know. I didn't even know about the |
||
// eslint-disable-next-line react/jsx-props-no-spreading | ||
{...defaultRendererProps} | ||
color={StyleUtils.getUserMentionTextColor(isOurMention)} | ||
style={StyleUtils.getUserMentionStyle(isOurMention)} | ||
onPress={() => showUserDetails(loginWhithoutLeadingAt)} | ||
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. Coming from #27951. When you press and hold on a mention, the context menu should open, but this doesn't work on native. |
||
> | ||
<TNodeChildrenRenderer tnode={props.tnode} /> | ||
</Text> | ||
</Tooltip> | ||
</Text> | ||
); | ||
}; | ||
|
||
MentionUserRenderer.propTypes = propTypes; | ||
MentionUserRenderer.displayName = 'MentionUserRenderer'; | ||
|
||
export default withCurrentUserPersonalDetails(MentionUserRenderer); |
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.
Coming from #18892:
style
was omitted from props and defined custom mention style.But this caused issue - some parent styles are not applied to child. i.e. bold from heading1.
More details about the root cause: #18892 (comment)