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

revert deprecating Color withOpacity / opacity #162069

Open
lukepighetti opened this issue Jan 23, 2025 · 13 comments
Open

revert deprecating Color withOpacity / opacity #162069

lukepighetti opened this issue Jan 23, 2025 · 13 comments
Labels
c: proposal A detailed proposal for a change to Flutter customer: crowd Affects or could affect many people, though not necessarily a specific customer. engine flutter/engine repository. See also e: labels. team-engine Owned by Engine team

Comments

@lukepighetti
Copy link
Contributor

lukepighetti commented Jan 23, 2025

Read through the code and migration guide but still having trouble supporting the end result here.

Axioms

  1. the vast majority of production codebases use .withOpacity(x), where x is 0.0–1.0 1
  2. the internal implementation is now .withOpacity(x) => .withAlpha((255.0*x).round()); 2
  3. the migration docs say to migrate .withOpacity(x) => .withValues(alpha: x); 3
  4. .withValues(alpha: x) is more verbose and less obvious than .withOpacity(x) 4
  5. designers communicate opacity as a percentage from 0%–100% not an integer from 0–255 5

Questions

  1. why doesn't the internal implementation match the migration docs
  2. why does withAlpha(x) take an int 0–255 and withValues(alpha: x) take a double 0–1.0
  3. why does .withOpacity need to be deprecated at all

Recommendations

  1. unify alpha to a double or an int across all color methods
  2. undeprecate withOpacity method
  3. undeprecate opacity getter
@lukepighetti lukepighetti changed the title undeprecate Color withOpacity / opacity revert deprecating Color withOpacity / opacity Jan 23, 2025
@maxfrees

This comment has been minimized.

@maxfrees
Copy link

why does .withOpacity need to be deprecated at all

@danagbemava-nc danagbemava-nc added in triage Presently being triaged by the triage team engine flutter/engine repository. See also e: labels. c: proposal A detailed proposal for a change to Flutter team-engine Owned by Engine team and removed in triage Presently being triaged by the triage team labels Jan 23, 2025
@kvermeille

This comment has been minimized.

@KRTirtho
Copy link

As a human being, I think perciving 0-1 is much easier than perciving 0-255. .withOpacity was an easier API and globally understood by all frontend/mobile/UI developers.
I'm guessing the change was made due to the confusion with Opacity widget's opacity. But, the method is in Color class so there's not much room for confusion

@Osmosis311

This comment has been minimized.

@kaankoken

This comment has been minimized.

@mit-mit
Copy link
Member

mit-mit commented Jan 23, 2025

Please do not add +1 comments; it adds noise. Rather, please use the "thumps up" button on the top comment to indicate "+1". More details here.

@arnab
Copy link

arnab commented Jan 23, 2025

Even if you don't revert it (I would prefer that too, for all the points already added), I would love to know why there is this discrepency:

why does withAlpha(x) take an int 0–255 and withValues(alpha: x) take a double 0–1.0

I see a lot of confusion in Flutter developers caused by this (like I have run into today). Alpha should either be 0..255, or 0..1, not "polymorphic" based on the context.

@bernaferrari
Copy link
Contributor

I also think withValues is a bad name, it should have been copyWith to be consistent with the framework (and the day data classes become a reality).

@JediPixels
Copy link

I agree, it's anti-intuitive

@hamed-rezaee
Copy link

The absence of method overloading is really noticeable here. 😐

@jezell
Copy link

jezell commented Jan 26, 2025

Color APIs really took a turn for the worse with the update. The lack of refinement with the API signatures has really overshadowed the P3 / HDR color support, which was an awesome addition.

@danagbemava-nc danagbemava-nc added the customer: crowd Affects or could affect many people, though not necessarily a specific customer. label Jan 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: proposal A detailed proposal for a change to Flutter customer: crowd Affects or could affect many people, though not necessarily a specific customer. engine flutter/engine repository. See also e: labels. team-engine Owned by Engine team
Projects
None yet
Development

No branches or pull requests