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 Edited Line Through Alignment #17781

Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
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
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,15 @@ const propTypes = {

const EditedRenderer = (props) => {
const defaultRendererProps = _.omit(props, ['TDefaultRenderer', 'style', 'tnode']);
const isPendingDelete = !!props.tnode.attributes.deleted;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think Boolean(props.tnode.attributes.deleted); might be a little clearer. You could also do this inline on line 26 instead of making a separate variable for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated


Copy link
Contributor

Choose a reason for hiding this comment

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

NAB: extra blank line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

return (
<Text
// eslint-disable-next-line react/jsx-props-no-spreading
{...defaultRendererProps}
fontSize={variables.fontSizeSmall}
color={themeColors.textSupporting}
style={[styles.alignItemsBaseline, editedLabelStyles]}
style={[editedLabelStyles, isPendingDelete && styles.offlineFeedback.deleted]}
>
{/* Native devices do not support margin between nested text */}
<Text
Expand Down
4 changes: 2 additions & 2 deletions src/pages/home/report/ReportActionItemFragment.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ const ReportActionItemFragment = (props) => {
// Only render HTML if we have html in the fragment
if (!differByLineBreaksOnly) {
const isPendingDelete = props.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE && props.network.isOffline;
const editedTag = props.fragment.isEdited ? '<edited></edited>' : '';
const editedTag = props.fragment.isEdited ? `<edited ${isPendingDelete ? 'deleted="true"' : ''}></edited>` : '';
Copy link
Contributor

Choose a reason for hiding this comment

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

NAB: Do we need ="true" here or can we just include deleted as boolean attribute?

Copy link
Contributor Author

@pubudu-ranasinghe pubudu-ranasinghe May 25, 2023

Choose a reason for hiding this comment

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

Yes, I've updated to use just the attribute key

const htmlContent = applyStrikethrough(html + editedTag, isPendingDelete);

return <RenderHTML html={props.source === 'email' ? `<email-comment>${htmlContent}</email-comment>` : `<comment>${htmlContent}</comment>`} />;
Expand All @@ -125,7 +125,7 @@ const ReportActionItemFragment = (props) => {
<Text
fontSize={variables.fontSizeSmall}
color={themeColors.textSupporting}
style={[styles.alignItemsBaseline, editedLabelStyles]}
style={[editedLabelStyles, ...props.style]}
>
<Text
selectable={false}
Expand Down
3 changes: 2 additions & 1 deletion src/styles/editedLabelStyles/index.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import display from '../utilities/display';
import flex from '../utilities/flex';

export default {...display.dInlineFlex};
export default {...display.dInlineFlex, ...flex.alignItemsBaseline};