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

TextField doesn't support diacritic "combining low line" (U+0331) below characters with a tail (g,j,p,q,y). #22104

Closed
imnasnainaec opened this issue Aug 7, 2020 · 4 comments · Fixed by #22149
Labels
bug 🐛 Something doesn't work component: text field This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@imnasnainaec
Copy link
Contributor

Current Behavior 😯

When typing or pasting g̲, j̲, p̲, q̲, or y̲, that is g/j/p/q/y with diacritic "combining low line" (U+0331), into a TextField, the underline is not visible.

Expected Behavior 🤔

The combining low line should be visible even below characters with a tail: g̲ j̲ p̲ q̲ y̲

Steps to Reproduce 🕹

Copy g̲ j̲ p̲ q̲ y̲ and paste into a demo TextField at https://material-ui.com/components/text-fields/.

Context 🔦

I am a programmer on an application used with endangered, indigenous, and minority languages, including Wakashan languages, which use g̲.

@imnasnainaec imnasnainaec added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Aug 7, 2020
@imnasnainaec
Copy link
Contributor Author

imnasnainaec commented Aug 7, 2020

The issue also exists with U+0328 Combining Ogonek and U+0323 Combining Dot Below diacritics, such as y̨ (used in Elfdalian), g̣ (used in Ojibwe), and j̣ and ŋ̣ (used for Inari Sami).

@joshwooding
Copy link
Member

joshwooding commented Aug 7, 2020

Seems like not resetting the line-height fixes this, note: the native input has the same issue. It seems the lowest line-height the combining low line is visible at is 22px.

--- a/packages/material-ui/src/InputBase/InputBase.js
+++ b/packages/material-ui/src/InputBase/InputBase.js
@@ -40,7 +40,6 @@ export const styles = (theme) => {
       // Mimics the default input display property used by browsers for an input.
       ...theme.typography.body1,
       color: theme.palette.text.primary,
-      lineHeight: '1.1876em', // Reset (19px), match the native input line-height
       boxSizing: 'border-box', // Prevent padding issue with fullWidth.
       position: 'relative',
       cursor: 'text',

Resetting was added here: #10346

image

Included ที่นั่น to show there's no regression of #20363

The reset was done when we didn't depend on theme.typography.body1 seems like it's safe to remove now.

@joshwooding joshwooding added bug 🐛 Something doesn't work component: text field This is the name of the generic UI component, not the React module! and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Aug 7, 2020
@oliviertassinari

This comment has been minimized.

@joshwooding
Copy link
Member

@imnasnainaec Did you want to make the change? It would be a great first contribution :)

@joshwooding joshwooding added the good first issue Great for first contributions. Enable to learn the contribution process. label Aug 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: text field This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants