Skip to content

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

jolexxa
jolexxa 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 jolexxa via fc42173 May 12, 2022 21:51
- [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
Member

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
Member

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