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

[v4] [core] fix(EditableText): do not use disabled text colors #4983

Merged
merged 1 commit into from Oct 22, 2021

Conversation

adidahiya
Copy link
Contributor

@adidahiya adidahiya commented Oct 22, 2021

Fixes #4974

Checklist

  • Includes tests
  • Update documentation

Changes proposed in this pull request:

I believe these color overrides were added in error; we don't want to render text with "disabled" appearance when disabled={true}; rather, that prop controls whether the interactive functionality is enabled on this component.

Reviewers should focus on:

Was there some other intention behind this code?

Screenshot

image

@blueprint-bot
Copy link

[v4] [core] fix(EditableText): do not use disabled text colors when disabled={true}

Previews: documentation | landing | table | modern colors demo

@adidahiya adidahiya changed the title [v4] [core] fix(EditableText): do not use disabled text colors when disabled={true} [v4] [core] fix(EditableText): do not use disabled text colors Oct 22, 2021
@adidahiya adidahiya merged commit 86c7c9d into next Oct 22, 2021
@adidahiya adidahiya deleted the ad/fix-editable-text-color branch October 22, 2021 16:18
@aycai
Copy link
Contributor

aycai commented Oct 22, 2021

The original change and intent was the placeholder text using the disabled text color and failing contrast. The placeholder color should be the only state that changes (which looks correct on the docs site), so if this is touching the disabled state then removing this is correct.

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.

None yet

3 participants