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.

Sign up for GitHub

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

Jump to bottom

Refactor MaterialStateProperty lerp functions #104507

Merged
merged 2 commits into from
Jun 2, 2022
Merged

Refactor MaterialStateProperty lerp functions #104507

merged 2 commits into from
Jun 2, 2022

Conversation

TahaTesser
Copy link
Member

@TahaTesser TahaTesser commented May 24, 2022 •
edited
Loading

fixes #104506
fixes #60555

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@flutter-dashboard flutter-dashboard bot added f: material design flutter/packages/flutter/material repository. f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels. labels May 24, 2022
HansMuller
HansMuller reviewed May 24, 2022
View reviewed changes
Copy link
Contributor

@HansMuller HansMuller left a comment

Choose a reason for hiding this comment

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

This is a nice improvement. Enshrining the special cases for BorderSide and OutlinedBorder in new public API doesn't seem like a good idea; better to create lerp functions for those types.

@HansMuller
Copy link
Contributor

By "better to create lerp functions for those types" I meant that it would be better to update BorderSide.lerp to handle nulls, and ... it looks like you can already define MaterialStateProperty.lerp in terms of ShapeBorder.lerp already?

@TahaTesser
Copy link
Member Author

By "better to create lerp functions for those types" I meant that it would be better to update BorderSide.lerp to handle nulls, and ... it looks like you can already define MaterialStateProperty.lerp in terms of ShapeBorder.lerp already?

Thank you, I understand it better now.

ShapeBorder.lerp requires casting to OutlinedBorder as a result I'm adding OutlinedBorder.lerp from #60555 and I will also update BorderSide.lerp to handle nulls.

@TahaTesser TahaTesser force-pushed the material_state_property_lerp branch from e442cb5 to beb3a8a Compare May 26, 2022 09:00
@TahaTesser
Copy link
Member Author

TahaTesser commented May 26, 2022 •
edited
Loading

@HansMuller
Thank you for the feedback, I've added OutlinedBorder.lerp function and link the issue for it above.

I decided to leave out making BorderSide.lerp support nullable as making it nullable requires adding ! everywhere this function is used in the source code. More classes use it as non-nullable than nullable so making it nullable will require lots of !, this could be done separately from this.

HansMuller
HansMuller approved these changes May 26, 2022
View reviewed changes
Copy link
Contributor

@HansMuller HansMuller left a comment

Choose a reason for hiding this comment

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

LGTM

packages/flutter/lib/src/painting/borders.dart Outdated Show resolved Hide resolved
@TahaTesser TahaTesser force-pushed the material_state_property_lerp branch from 0fb6eff to c66f515 Compare May 27, 2022 10:12
@fluttergithubbot
Copy link
Contributor

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

@TahaTesser TahaTesser force-pushed the material_state_property_lerp branch from c66f515 to 40ae46b Compare May 30, 2022 08:59
@TahaTesser
Copy link
Member Author

Google testing — No tests has been executed due to internal error. Ask a Googler to investigate.

Trying again

@TahaTesser
Refactor MaterialStateProperty lerp functions
0f4bfb4 
restore border and shape lerp function

Add `OutlinedBorder.lerp`

Resolve conflicts
@TahaTesser
Use curly_braces_in_flow_control_structures
fd97464
@TahaTesser TahaTesser force-pushed the material_state_property_lerp branch from 40ae46b to fd97464 Compare May 31, 2022 08:45
@TahaTesser
Copy link
Member Author

@HansMuller
The Google Test is still failing, can you please check what's the reason?

@HansMuller
Copy link
Contributor

I asked on hackers-infra, I don't understand why "Google Testing" keeps failing. The details link just points to an empty FROB page.

@TahaTesser
Copy link
Member Author

Thanks for raising this. (I cannot access the details without credentials)

@keyonghan
Copy link
Contributor

It's not showing in go/flutter-rob as it had already been tested 8 times. @HansMuller
Feel free to land it if you are comfortable and we can verify it in postsubmit.

@CaseyHillers filed b/234758732 to track.

@HansMuller
Copy link
Contributor

@keyonghan - OK, I'm going to land this. It should be safe.

@HansMuller HansMuller merged commit 8780436 into flutter:master Jun 2, 2022
@TahaTesser TahaTesser deleted the material_state_property_lerp branch June 3, 2022 11:24
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 3, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 3, 2022
@engine-flutter-autoroll
87804363e Refactor MaterialStateProperty lerp functions (flutter/fl…
aecf68b 
…utter#104507)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 3, 2022
Closed
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 4, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 4, 2022
@engine-flutter-autoroll
87804363e Refactor MaterialStateProperty lerp functions (flutter/fl…
a1bfe21 
…utter#104507)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 4, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 4, 2022
@engine-flutter-autoroll
87804363e Refactor MaterialStateProperty lerp functions (flutter/fl…
35f4535 
…utter#104507)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 5, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 5, 2022
@engine-flutter-autoroll
87804363e Refactor MaterialStateProperty lerp functions (flutter/fl…
5eb036a 
…utter#104507)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 5, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 5, 2022
@engine-flutter-autoroll
87804363e Refactor MaterialStateProperty lerp functions (flutter/fl…
a04751b 
…utter#104507)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 5, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 5, 2022
@engine-flutter-autoroll
87804363e Refactor MaterialStateProperty lerp functions (flutter/fl…
def092f 
…utter#104507)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 6, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 6, 2022
@engine-flutter-autoroll
87804363e Refactor MaterialStateProperty lerp functions (flutter/fl…
fdc6cb1 
…utter#104507)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 6, 2022
@engine-flutter-autoroll
87804363e Refactor MaterialStateProperty lerp functions (flutter/fl…
5a91801 
…utter#104507)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 6, 2022
@engine-flutter-autoroll
87804363e Refactor MaterialStateProperty lerp functions (flutter/fl…
9b4b3a2 
…utter#104507)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 6, 2022
@engine-flutter-autoroll
87804363e Refactor MaterialStateProperty lerp functions (flutter/fl…
ccd4d81 
…utter#104507)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 6, 2022
@engine-flutter-autoroll
87804363e Refactor MaterialStateProperty lerp functions (flutter/fl…
9cc807a 
…utter#104507)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 7, 2022
@engine-flutter-autoroll
87804363e Refactor MaterialStateProperty lerp functions (flutter/fl…
54edcae 
…utter#104507)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 7, 2022
@engine-flutter-autoroll
87804363e Refactor MaterialStateProperty lerp functions (flutter/fl…
83206ae 
…utter#104507)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 7, 2022
@engine-flutter-autoroll
87804363e Refactor MaterialStateProperty lerp functions (flutter/fl…
5a379fa 
…utter#104507)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 7, 2022
@engine-flutter-autoroll
87804363e Refactor MaterialStateProperty lerp functions (flutter/fl…
40af77d 
…utter#104507)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 7, 2022
@engine-flutter-autoroll
87804363e Refactor MaterialStateProperty lerp functions (flutter/fl…
bcb63c7 
…utter#104507)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 7, 2022
@engine-flutter-autoroll
87804363e Refactor MaterialStateProperty lerp functions (flutter/fl…
f475fef 
…utter#104507)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 7, 2022
@engine-flutter-autoroll
87804363e Refactor MaterialStateProperty lerp functions (flutter/fl…
b1667f9 
…utter#104507)
This was referenced Jun 7, 2022
Closed
Closed
camsim99 pushed a commit to camsim99/flutter that referenced this pull request Aug 10, 2022
@TahaTesser @camsim99
Refactor MaterialStateProperty lerp functions (flutter#104507)
c772108
@TahaTesser TahaTesser mentioned this pull request Aug 26, 2022
Merged
8 tasks
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2022
@engine-flutter-autoroll
87804363e Refactor MaterialStateProperty lerp functions (flutter/fl…
bff60a1 
…utter#104507)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Aug 30, 2022
@engine-flutter-autoroll
8780436 Refactor MaterialStateProperty lerp functions (flutter/flut…
5a7c36d 
…ter#104507)
@TahaTesser TahaTesser mentioned this pull request Sep 14, 2022
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HansMuller HansMuller HansMuller approved these changes

Assignees
No one assigned
Labels
f: material design flutter/packages/flutter/material repository. f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Refactor MaterialStateProperty lerp functions Need a definition of OutlinedShape.lerp
4 participants
@TahaTesser @HansMuller @fluttergithubbot @keyonghan