Skip to content

Conversation

@kyledurand
Copy link
Member

WHY are these changes introduced?

Fixes #182

WHAT is this pull request doing?

I think webkit overrides the color value of disabled inputs. We were using current color here before because I think legacy Polaris had different colors for different disabled states i.e. error. Now they're all using --p-text-disabled

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Copy-paste this code in playground/Playground.tsx:
import React from 'react';

import {Page, TextField} from '../src';

export function Playground() {
  return (
    <Page title="Playground">
      <TextField
        disabled
        label="some value"
        value="some value"
        onChange={(value) => console.log(value)}
      />
    </Page>
  );
}

🎩 checklist

@kyledurand kyledurand self-assigned this Jul 28, 2021
@kyledurand kyledurand requested a review from nicolleromero July 28, 2021 13:20
@kyledurand kyledurand force-pushed the text-field_fix-webkit-disabled-text-color branch from 1470fe9 to da983fa Compare July 28, 2021 13:21
@github-actions
Copy link
Contributor

github-actions bot commented Jul 28, 2021

size-limit report

Path Size
cjs 142.12 KB (0%)
esm 95.85 KB (0%)
esnext 138.86 KB (+0.01% 🔺)
css 33.56 KB (+0.01% 🔺)

@kyledurand kyledurand force-pushed the text-field_fix-webkit-disabled-text-color branch from da983fa to 1b8b2ab Compare July 28, 2021 13:29
Copy link
Contributor

@nicolleromero nicolleromero left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

Copy link
Contributor

@adriancorcoran adriancorcoran left a comment

Choose a reason for hiding this comment

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

Hey folks, sorry to block (not sure what your conventions are when reviewing :) ), but this is what I saw in Safari on desktop and mobile (all other browsers checked out ok):

Here's the exact list of steps I followed:

  1. dev clone polaris-react

  2. dev u

  3. Added the following code to the playground/Playground.tsx file

    import React from 'react';
    
    import {Page, TextField} from '../src';
    
    export function Playground() {
      return (
        <Page title="Playground">
          <TextField
            disabled
            label="some value"
            value="some value"
            onChange={(value) => console.log(value)}
          />
        </Page>
      );
    }
  4. yarn dev

  5. go to http://192.168.86.105:6006/?path=/story/playground-playground--playground in Safari

Maybe I missed something? Let me know if I can help further :)

@kyledurand
Copy link
Member Author

Looks like you might have missed git checkout text-field_fix-webkit-disabled-text-color If not then can you share the safari version you're using @adriancorcoran

Copy link
Contributor

@adriancorcoran adriancorcoran left a comment

Choose a reason for hiding this comment

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

Oh god how embarrassing!! :) New repo and tophatting procedure, sorry about that folks!

✅ We're all good

@kyledurand kyledurand merged commit 66b1692 into main Jul 29, 2021
@kyledurand kyledurand deleted the text-field_fix-webkit-disabled-text-color branch July 29, 2021 15:57
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