Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

Fix material colors to match previous version better #13666

Merged
merged 9 commits into from
Feb 6, 2021
Merged

Conversation

PureWeen
Copy link
Contributor

@PureWeen PureWeen commented Feb 5, 2021

Description of Change

  • fixed android so the disabled colors are correct
  • fixed iOS so if you set the placeholder color and the control is disabled it'll set the color correctly
  • fixed the underline on android so when not focused it styles correctly relative to the place holder
  • fixed the underline on android to shade correctly when not focused
  • Fixed it so if you have a button with an image that gets pushed onto the stack the background color will render correctly

Issues Resolved

Platforms Affected

  • Android

Behavioral/Visual Changes

  • on iOS when the control is disabled the PH color now sets correctly

Testing Procedure

  • Visual Gallery Included tests so you can compare Colors

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)

@PureWeen PureWeen added this to the 5.0.0 milestone Feb 5, 2021
@PureWeen PureWeen added this to To Fix in 5.0.0 SR 3 via automation Feb 5, 2021
@PureWeen PureWeen moved this from To Fix to PR Needs Review in 5.0.0 SR 3 Feb 5, 2021
@jsuarezruiz
Copy link
Contributor

@PureWeen Should we mark this PR with the blocker flag?

Copy link
Contributor

@jsuarezruiz jsuarezruiz left a comment

Choose a reason for hiding this comment

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

Android
fix13666
iOS
Captura de pantalla 2021-02-05 a las 9 47 24

Copy link
Contributor

@jsuarezruiz jsuarezruiz left a comment

Choose a reason for hiding this comment

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

When the Entry is disabled in iOS, the bottom line is dashed, is an expected behavior?

@PureWeen
Copy link
Contributor Author

PureWeen commented Feb 5, 2021

@jsuarezruiz looking at 4.8 that hasn't changed so we can fix that with a different PR if we need to

image

@PureWeen PureWeen added the DO-NOT-MERGE-!!! 🛑 This is in progress and needs to be updated before it can be merged. label Feb 5, 2021
Copy link
Member

@Redth Redth left a comment

Choose a reason for hiding this comment

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

Other than the underline differences @jsuarezruiz noticed, lgtm!

@PureWeen PureWeen removed the DO-NOT-MERGE-!!! 🛑 This is in progress and needs to be updated before it can be merged. label Feb 5, 2021
// an issue with setting background color and having an image set
if (Enabled && Element.ImageSource != null)
{
Enabled = false;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Redth or @jsuarezruiz if you have any better thoughts or tricks to address this issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Material-UI bug on several ui controls
3 participants