Skip to content

Conversation

war-in
Copy link
Collaborator

@war-in war-in commented Aug 13, 2025

Details

This PR adds support for borderRadius prop in mentions styles on iOS & Android

image

Related Issues

Expensify/App#19299

Manual Tests

iOS & Android:

  1. Verify if singleline mentions have rounded edges
  2. Verify if multiline mentions have rounded edges only at the beginning and the end
  3. Verify mentions work correctly in blockquotes

Linked PRs

@war-in war-in changed the title Add mention-user border radius [iOS] Add mention-user border radius Aug 13, 2025
@war-in war-in changed the title [iOS] Add mention-user border radius [iOS] Enable borderRadius in all mention types Aug 14, 2025
Comment on lines 114 to 120
CGFloat marginLeft = _markdownUtils.markdownStyle.blockquoteMarginLeft;
CGFloat borderWidth = _markdownUtils.markdownStyle.blockquoteBorderWidth;
CGFloat paddingLeft = _markdownUtils.markdownStyle.blockquotePaddingLeft;
CGFloat shift = marginLeft + borderWidth + paddingLeft;

fragmentTextBounds.origin.x -= (paddingLeft + borderWidth) + shift * ([_depth unsignedIntValue] - 1);
fragmentTextBounds.size.width = borderWidth + shift * ([_depth unsignedIntValue] - 1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like the blockquote logic is now duplicated, can we somehow dedupe it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It was already :/
I could create a helper function to get the shift to make it look better but the logic would stay unchanged

I think we need it to be that way because boundingRect is also used in renderingSurfaceBounds

@war-in war-in requested a review from tomekzaw August 19, 2025 14:49
@war-in war-in changed the title [iOS] Enable borderRadius in all mention types [iOS & Android] Enable borderRadius in all mention types Aug 20, 2025
@war-in war-in marked this pull request as ready for review August 20, 2025 14:48
@war-in war-in requested a review from jmusial August 21, 2025 10:24
@war-in
Copy link
Collaborator Author

war-in commented Aug 22, 2025

I fixed last known iOS issue in this commit - 28b6a13

Before
image

After
image

@war-in war-in requested a review from tomekzaw August 22, 2025 09:33
Copy link
Collaborator

@jmusial jmusial left a comment

Choose a reason for hiding this comment

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

Dunno if those are connected to this PR, or just existing bugs. In all cases, it's just on mobile devices

  1. android, switching to single line breaks highlights :(
Screen.Recording.2025-08-22.at.13.04.20.mov
  1. android, Cursor not visible when within codeblock or mention
Screen.Recording.2025-08-22.at.13.08.38.mov

mentionHere: {
color: 'green',
backgroundColor: 'lime',
borderRadius: 5,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it me or default borderRadius for android and web look different ?

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, saw that too. Not sure why the same radius is smaller on Android 🤔

@tomekzaw any ideas?

@jmusial
Copy link
Collaborator

jmusial commented Aug 22, 2025

Very much a nitpick, but when having a multiline mention starting bottom border radius and finish top, should not apply. Same issue for web

image

@jmusial
Copy link
Collaborator

jmusial commented Aug 22, 2025

Na ios dzieją się rzeczy niestworzone :( Ale to też single - line

to jest 17.5, nie wiem czy to ma znaczenie

Simulator.Screen.Recording.-.iPhone.15.Pro.-.2025-08-22.at.14.34.03.mp4

@war-in
Copy link
Collaborator Author

war-in commented Aug 26, 2025

Very much a nitpick, but when having a multiline mention starting bottom border radius and finish top, should not apply. Same issue for web

We can try to fix it in a separate PR but I think we need to consult with the design team. I'll leave it as an open idea because it's not in the scope of this PR

Thanks for noticing!

@war-in
Copy link
Collaborator Author

war-in commented Aug 26, 2025

  1. android, switching to single line breaks highlights :(

@jmusial should be fixed with the latest commit 🎉 98dafda

  1. android, Cursor not visible when within codeblock or mention

this is expected behaviour. We discussed it with the design team and there is no plan to support it in the near future

@tomekzaw
Copy link
Collaborator

  1. android, Cursor not visible when within codeblock or mention

For the context, this was discussed on Slack: https://swmansion.slack.com/archives/C01GTK53T8Q/p1755699466781499

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants