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

#201 - Null-Safety support #209

Merged
merged 12 commits into from Mar 10, 2021
Merged

#201 - Null-Safety support #209

merged 12 commits into from Mar 10, 2021

Conversation

brenodt
Copy link
Contributor

@brenodt brenodt commented Mar 8, 2021

This PR aims to bring null-safety support to the package - as per #201 request.

The approach taken was to change as little as possible in terms of logic:

  • Places where @required tag was used were replaced with required keyword and became non-nullable.
  • Parameters that are checked for nullability somewhere down the line were set as nullable.
  • If none of these two scenarios applied, used a default value.

On slider.dart and range_slider.dart, the onPanUpdate method declarations have had to be updated, as context.findRenderObject() no longer yields a RenderBox
(lib/src/widget/slider.dart:143, lib/src/widget/range_slider.dart:168). Instead, the tap position is now using DragUpdateDetails.localPosition.

Secondly, I couldn't ascertain how to regenerate the lib/generated/i18n.dart file, but I don't see it being used anywhere. Maybe it should be dropped?

@florent37 I hope this helps bringing the package to Null-Safety, let me know if you think I should change anything. Thanks in advance!

@7flash
Copy link

7flash commented Mar 9, 2021

Please merge @florent37

@7flash
Copy link

7flash commented Mar 9, 2021

Screenshot from 2021-03-09 14-39-18
I am getting this error when trying to run app depending on your branch brenodt:158-null-safety-support

@7flash
Copy link

7flash commented Mar 9, 2021

This works - please fix @brenodt

  static double? embossDepth(BuildContext context) {
    return currentTheme(context).depth != null
        ? -(currentTheme(context).depth!.abs())
        : null;
  }

@brenodt
Copy link
Contributor Author

brenodt commented Mar 9, 2021

@7flash Thanks for the review, fix made.

@florent37
Copy link
Member

wow thanks a lot guys, I merge it !

@florent37 florent37 merged commit 717efd2 into Idean:master Mar 10, 2021
@florent37
Copy link
Member

I will publish a new verison <3

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