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 variant outlined's border color doesn't change when focus on mobile #16149

Closed
2 tasks done
punnadittr opened this issue Jun 11, 2019 · 7 comments · Fixed by #16266
Closed
2 tasks done

Textfield variant outlined's border color doesn't change when focus on mobile #16149

punnadittr opened this issue Jun 11, 2019 · 7 comments · Fixed by #16266
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

@punnadittr
Copy link

  • This is not a v0.x issue.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior 🤔

The outlined border color should change from grey to primary color (when focus) across all breakpoints

Current Behavior 😯

On mobile (both devtools and android chrome), the border color won't change to primary color when focusing

Steps to Reproduce 🕹

It occurs in my own project but I noticed that it occurs on Mui doc as well

Link: https://material-ui.com/components/text-fields/

  1. Go to Outlined section
  2. Toggle developer tools and set to any mobile device
  3. Click on any of the outlined fields

Your Environment 🌎

Tested on Android Chrome with my own phone and Linux Chrome

Tech Version
Material-UI v4.1.0
React 16.8.6
Browser Chrome (Linux) 75.0.3770.80
TypeScript
etc. Chrome (Android) 75.0.3770.67
@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 11, 2019

@punnadittr Thanks for reporting this regression between v3.9.3 and v4.0.2. This seems to be a specificity issue in JSS (cc @kof) but I can't track down when it happened.

The following diff fixes the problem:

diff --git a/packages/material-ui/src/OutlinedInput/OutlinedInput.js b/packages/material-ui/src/OutlinedInput/OutlinedInput.js
index e15bbea9a..6799212d4 100644
--- a/packages/material-ui/src/OutlinedInput/OutlinedInput.js
+++ b/packages/material-ui/src/OutlinedInput/OutlinedInput.js
@@ -18,8 +18,9 @@ export const styles = theme => {

   return {
     /* Styles applied to the root element. */
     root: {
       position: 'relative',
       '& $notchedOutline': {
         borderColor,
       },
       '&:hover $notchedOutline': {
         borderColor: theme.palette.text.primary,
-        // Reset on touch devices, it doesn't add specificity
-        '@media (hover: none)': {
+      },
+      // Reset on touch devices, it doesn't add specificity
+      '@media (hover: none)': {
+        '&:hover $notchedOutline': {
           borderColor,
         },
       },
       '&$focused $notchedOutline': {
         borderColor: theme.palette.primary.main,
         borderWidth: 2,
       },

I can't find any other occurrences of the problem in the codebase, I believe this patch is safe to apply. Do you want to submit a pull request?

@oliviertassinari oliviertassinari added 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. labels Jun 11, 2019
@kof
Copy link
Contributor

kof commented Jun 11, 2019

Is it this cssinjs/jss#1045 ?

@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 11, 2019

I'm wondering if the @media doesn't change the code paths. If I had to estimate a probability, I would say 50%.

@arminydy
Copy link
Contributor

@oliviertassinari
I have just migrated my project from v3 to v4, so far so good, except this is the obvious one. This issue is not only happening on mobile. In a nutshell, Textfield variant outlined's border color is not changing on focus. @mediahover is overriding it.
By any chance will it be fixed any time soon? thanks.

@kof
Copy link
Contributor

kof commented Jun 17, 2019

a clear reduced reproduction anyone with JSS core?

@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 17, 2019

This issue is not only happening on mobile.

@arminydy Sounds like you have an issue in your environment.

By any chance will it be fixed any time soon? thanks.

If you want to give it a shoot, we will be happy to review your pull request :). The issue was labeled "good first issue". It's low-hanging fruit. Meaning, the core team won't be working on it, so we can work on issues we are uniquely positioned to address.

arminydy added a commit to arminydy/material-ui that referenced this issue Jun 17, 2019
-Desc: highlight TextField varient outlines border color on focus event on mobile
@arminydy
Copy link
Contributor

@oliviertassinari I followed your guidance, it works like a charm now :) , I will appreciate if you could have a quick look on this PR. Thanks.

arminydy added a commit to arminydy/material-ui that referenced this issue Jun 17, 2019
-Desc: highlight TextField varient outlines border color on focus event on mobile
oliviertassinari pushed a commit that referenced this issue Jun 18, 2019
* -ticket: #16149 (comment)

-Desc: highlight TextField varient outlines border color on focus event on mobile

* -ticket: #16149 (comment)

-Desc: highlight TextField varient outlines border color on focus event on mobile

* reduce specificity
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.

4 participants