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

Feature/styled icon supporting decorators #159

Merged
merged 11 commits into from
Jan 22, 2024

Conversation

tilucasoli
Copy link
Collaborator

@tilucasoli tilucasoli commented Jan 18, 2024

Issue ticket number

closes #156

Describe your changes

  • Create additional test cases for both Box and StyledBox;
  • Add RenderWidgetDecorators to the MixedIcon widget tree, enabling the usege of decorators to StyledIcon;
  • Introduce a new attribute, isInheritable, in the Attribute class;
  • Implement a new constructor, MixData.inherited, in the MixData class. This constructor should accept a context, retriece MixData from it, and remove attributes where the isInheritable property returns false;
  • Establish a rule that decorators cannot be inherited by any children;

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Test (adding missing tests, refactoring tests; no production code change)
  • New Feature (non-breaking change which adds functionality)
  • Docs (changes to the documentation)

Checklist before requesting a review

  • I have performed a self-review of my code
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works

@tilucasoli tilucasoli marked this pull request as ready for review January 18, 2024 18:40
@@ -62,6 +62,9 @@ class RenderWidgetDecorators extends StatelessWidget {
final decorator = decoratorMap.remove(decoratorType);
if (decorator == null) continue;
current = decorator.build(mix, current);

// Remove the decorator from the MixData, because it can be applied only once.
mix.removeType(decoratorType);
Copy link
Member

Choose a reason for hiding this comment

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

@tilucasoli I believe we might have to take another approach than mutating the attributes of the MixData; I think we might have to create an immutable class and pass it down to the child.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, it's really important to remember!
Thinking about possible solutions, I identified two. The first one is to propagate a new value in RenderWidgetDecorators using MixProvider. The second possibility is to create a new MixData without the decorator attributes when the existing MixData is inherited from MixProvider. My favorite solution is the second one, because adding a new widget to the widget tree unnecessarily doesn't seem like a good ideia.

remove RenderWidgetDecorators when there isnt decorators attributes in style
import '../../factory/mix_provider_data.dart';
import 'icon_attribute.dart';
import 'icon_spec.dart';
import '../../../mix.dart';
Copy link
Member

Choose a reason for hiding this comment

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

Just be aware of this, vscode does this all the time to consolidate the exports. I wonder if there is a way to make sure it does not use mix.dart as an import

Copy link
Member

@leoafarias leoafarias left a comment

Choose a reason for hiding this comment

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

LGTM

@leoafarias leoafarias merged commit 4bbd2be into main Jan 22, 2024
2 checks passed
@leoafarias leoafarias deleted the feature/styled-icon-supporting-decorators branch January 22, 2024 21:21
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.

StyledIcon doesn't support decorators attributes
2 participants