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

feat!: v3.0.0 #45

Merged
merged 5 commits into from May 13, 2022
Merged

feat!: v3.0.0 #45

merged 5 commits into from May 13, 2022

Conversation

felangel
Copy link
Contributor

Description

Type of Change

  • ✨ New feature (non-breaking change which adds functionality)
  • 🛠️ Bug fix (non-breaking change which fixes an issue)
  • ❌ Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 Code refactor
  • ✅ Build configuration change
  • 📝 Documentation
  • 🗑️ Chore

definitelyokay
definitelyokay previously approved these changes May 12, 2022
alestiago
alestiago previously approved these changes May 12, 2022
This was linked to issues May 12, 2022
@felangel felangel dismissed stale reviews from alestiago and definitelyokay via fc42173 May 12, 2022 21:51
Co-authored-by: Marcos Sevilla <31174242+marcossevilla@users.noreply.github.com>
.github/workflows/main.yaml Outdated Show resolved Hide resolved
- [sized_box_shrink_expand](https://dart-lang.github.io/linter/lints/sized_box_shrink_expand.html)
- [unnecessary_constructor_name](https://dart-lang.github.io/linter/lints/unnecessary_constructor_name.html)
- [unnecessary_late](https://dart-lang.github.io/linter/lints/unnecessary_late.html)
- [use_colored_box](https://dart-lang.github.io/linter/lints/use_colored_box.html)

Choose a reason for hiding this comment

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

<3

Copy link

@orestesgaolin orestesgaolin left a comment

Choose a reason for hiding this comment

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

giphy (1)

- [conditional_uri_does_not_exist](https://dart-lang.github.io/linter/lints/conditional_uri_does_not_exist.html)
- [secure_pubspec_urls](https://dart-lang.github.io/linter/lints/secure_pubspec_urls.html)
- [sized_box_shrink_expand](https://dart-lang.github.io/linter/lints/sized_box_shrink_expand.html)
- [unnecessary_constructor_name](https://dart-lang.github.io/linter/lints/unnecessary_constructor_name.html)
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this apply to dartdoc references?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you mean? 👀

Copy link
Contributor

@jeroen-meijer jeroen-meijer left a comment

Choose a reason for hiding this comment

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

:shipit:

@felangel felangel merged commit 6506c44 into main May 13, 2022
@felangel felangel deleted the feat/v3.0.0 branch May 13, 2022 16:18
@DFelten
Copy link

DFelten commented May 19, 2022

With the new lint rule use_colored_box there is one small problem. What should we do if the color is optional? Currently we're using a Container with the color for such case.

But now this is not allowed. We would need to return different widgets here if the color is null. Or is there a better way here?

@marcossevilla
Copy link

With the new lint rule use_colored_box there is one small problem. What should we do if the color is optional? Currently we're using a Container with the color for such case.

But now this is not allowed. We would need to return different widgets here if the color is null. Or is there a better way here?

I would setup a default color to that widget 👍 if that doesn't help, you can share a sample of how you have that widget set up so we can suggest a better approach

@DFelten
Copy link

DFelten commented May 19, 2022

It's currently a simple carousel with an optional background color. The carousel usually has no background so that the color of the widget on which it is displayed is used. However, there are cases where it should have its own background.

Small snippet (backgroundColor is optional):

  Widget build(BuildContext context) {
    return Container(
      color: backgroundColor,
      child: CarouselSlider(
        options: CarouselOptions(
          onPageChanged: (index, reason) => onItemChanged?.call(index),
          viewportFraction: viewportFraction,
          aspectRatio: aspectRatio,
          height: height,
          pageSnapping: false,
          scrollPhysics: TMXPageScrollPhysics.bookPageScrolling(viewportFraction: viewportFraction),
        ),
        items: items.map((T item) => itemBuilder(item)).toList(),
      ),
    );
  }

@marcossevilla
Copy link

if you go to the container's color rabbit hole, you'll see the default value is Colors.transparent, so you can do:

  Widget build(BuildContext context) {
    return DecoratedBox(
      color: backgroundColor ?? Colors.transparent,
      child: CarouselSlider(
        options: CarouselOptions(
          onPageChanged: (index, reason) => onItemChanged?.call(index),
          viewportFraction: viewportFraction,
          aspectRatio: aspectRatio,
          height: height,
          pageSnapping: false,
          scrollPhysics: TMXPageScrollPhysics.bookPageScrolling(viewportFraction: viewportFraction),
        ),
        items: items.map((T item) => itemBuilder(item)).toList(),
      ),
    );
  }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: Add use_super_parameters Consider enable these lints to rules
10 participants