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

[HOLD facebook/react-native#37397][$1000] Back ticks aren't centered vertically #8132

Closed
mvtglobally opened this issue Mar 14, 2022 · 96 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Monthly KSv2

Comments

@mvtglobally
Copy link

mvtglobally commented Mar 14, 2022

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Action Performed:

  1. Send few messages with back ticks

Expected Result:

Back ticks should be centered vertically

Actual Result:

Back ticks aren't centered vertically

Workaround:

unknown

Platform:

Where is this issue occurring?

  • iOS

Version Number: 1.1.42-2
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Image from iOS (11)

Expensify/Expensify Issue URL:
Issue reported by: @AndrewGable
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1646420349264159
Upwork: https://www.upwork.com/jobs/~01be27e71047382c31

View all open jobs on GitHub

@MelvinBot
Copy link

Triggered auto assignment to @stitesExpensify (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@stitesExpensify stitesExpensify added the External Added to denote the issue can be worked on by a contributor label Mar 15, 2022
@MelvinBot
Copy link

Triggered auto assignment to @mallenexpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@botify botify removed the Daily KSv2 label Mar 16, 2022
@MelvinBot MelvinBot added the Weekly KSv2 label Mar 16, 2022
@MelvinBot
Copy link

Triggered auto assignment to Contributor-plus team member for initial proposal review - @Santhosh-Sellavel (Exported)

@MelvinBot MelvinBot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Mar 16, 2022
@MelvinBot
Copy link

Current assignee @stitesExpensify is eligible for the Exported assigner, not assigning anyone new.

@mallenexpensify
Copy link
Contributor

@Phantom-2
Copy link

Proposal

in src/styles/styles.js

        code: {
            ...baseCodeTagStyles,
            ...codeStyles.codeTextStyle,
            paddingLeft: 5,
            paddingRight: 5,
            fontFamily: fontFamily.MONOSPACE,
            fontSize: 13,
+           paddingTop:1,

        },

        img: {
            borderColor: themeColors.border,
            borderRadius: variables.componentBorderRadiusNormal,
            borderWidth: 1,
        },

Simulator Screen Shot - iPhone 13 - 2022-03-16 at 13 11 28

@mallenexpensify
Copy link
Contributor

@Phantom-2 , apologies for the delay, @Santhosh-Sellavel can you please review the proposal above?

@Santhosh-Sellavel
Copy link
Collaborator

Sorry I didn’t leave a comment earlier.
This would make some unintended style break in other platforms.

@Santhosh-Sellavel
Copy link
Collaborator

Also, I tried to reproduce this on my end today, it's not occurring for me.

Can you check on the latest staging or production build, is the issue still occurring for you?
@AndrewGable

If reproducible, share me device modal & iOS version, thanks!

cc: @mallenexpensify @stitesExpensify

@AndrewGable
Copy link
Contributor

@mvtglobally - Can you have some testers try to reproduce? If if it's not reproducible, agree let's close.

@mountiny mountiny changed the title Back ticks aren't centered vertically [$250] Back ticks aren't centered vertically Mar 31, 2022
@stitesExpensify
Copy link
Contributor

@mvtglobally was this reproducible?

@MelvinBot MelvinBot removed the Overdue label Apr 4, 2022
@Santhosh-Sellavel
Copy link
Collaborator

@mvtglobally Any update on this?

@melvin-bot melvin-bot bot added the Overdue label Apr 19, 2022
@mallenexpensify
Copy link
Contributor

Pinged @mvtglobally in NewDot to take a look

@melvin-bot melvin-bot bot removed the Overdue label Apr 20, 2022
@mvtglobally
Copy link
Author

checking, Sorry for the delay. for some reason I missed this one.

@mvtglobally
Copy link
Author

mvtglobally commented Apr 20, 2022

Looks like I was unsubscribed from this issue alerts. I did not mean this.
Trying this right now, I am not able to use back-ticks on the multiline or paragraphs. If you use straight text, it is looking aligned

Screen Shot 2022-04-20 at 12 30 38 PM

@stitesExpensify stitesExpensify removed the Reviewing Has a PR in review label Apr 24, 2023
@melvin-bot melvin-bot bot added the Overdue label May 25, 2023
@sonialiap
Copy link
Contributor

Expensify/react-native#43 is still in process so this one's on hold

@melvin-bot melvin-bot bot removed the Overdue label May 25, 2023
@melvin-bot melvin-bot bot added the Overdue label Jun 26, 2023
@sonialiap
Copy link
Contributor

@roryabraham Expensify/react-native#43 was closed, did the solution get moved to facebook/react-native#37397? I'm wondering if this issue should still be on hold for one of those PRs

@melvin-bot melvin-bot bot removed the Overdue label Jun 26, 2023
@fabioh8010
Copy link
Contributor

@sonialiap @roryabraham Yes, solution was moved to facebook/react-native#37397. Once that PR get approved and merged we will be able to fix this one. We can take either one of the following paths:

  1. Wait for Add substring measurements for Text facebook/react-native#37397 to fix this issue.
  2. Proceed with my PR [HOLD] fix: improve mobile's inline code block font and alignment #15367 which is on HOLD for now. My PR is just a temporary workaround and the fix is not visually perfect, so it may not be the best approach to follow.
  3. Based on this discussion on Slack, simplify the code block styling in a way that won't cause the issue anymore, but it's a design change and needs approval from the design team.
  4. Explore other solutions on native side like the one I suggested, that I ended up stopping the investigation because Inline code block support react-native#43 was being worked on.

@melvin-bot melvin-bot bot added the Overdue label Aug 1, 2023
@stitesExpensify
Copy link
Contributor

I think we should still just wait for facebook/react-native#37397. This isn't that big of a deal.

@melvin-bot melvin-bot bot removed the Overdue label Aug 2, 2023
@stitesExpensify
Copy link
Contributor

cc @shawnborton in case you feel differently and we should really change this soon. In which case 3 from this comment would be a potential good solution.

@shawnborton
Copy link
Contributor

I am cool with waiting.

@melvin-bot melvin-bot bot added the Overdue label Sep 4, 2023
@sonialiap
Copy link
Contributor

No update on facebook/react-native#37397

@melvin-bot melvin-bot bot removed the Overdue label Sep 4, 2023
@melvin-bot melvin-bot bot added the Overdue label Oct 5, 2023
@sonialiap
Copy link
Contributor

No update

@melvin-bot melvin-bot bot removed the Overdue label Oct 6, 2023
@melvin-bot melvin-bot bot added the Overdue label Nov 6, 2023
@stitesExpensify
Copy link
Contributor

No update. I think we should just close this for now. Thoughts?

@trjExpensify
Copy link
Contributor

👋 Can we put the repo/issue we're holding on in the issue title please? Coming here from a dupe and it's easier to follow the dependencies when that's present in the title. Thanks!

@stitesExpensify stitesExpensify changed the title [HOLD][$1000] Back ticks aren't centered vertically [HOLD facebook/react-native#37397][$1000] Back ticks aren't centered vertically Nov 16, 2023
@melvin-bot melvin-bot bot added the Overdue label Dec 8, 2023
@sonialiap
Copy link
Contributor

@stitesExpensify do we have any way to push the upstream PR forward? If not, is it worth keeping our issue open when the /react-native hasn't been commented on since June?

@melvin-bot melvin-bot bot removed the Overdue label Dec 11, 2023
@stitesExpensify
Copy link
Contributor

do we have any way to push the upstream PR forward?

Not really unless we want to divert expert contributor resources to this (which we shouldn't do IMO). I think we should just close this for now and come back to it later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Monthly KSv2
Projects
None yet
Development

No branches or pull requests