feat: Callbacks in HudButtonComponent constructor and ViewportMargin mixin to avoid code duplication
#1685
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
spydon
requested review from
erickzanardo,
luanpotter,
renancaraujo and
wolfenrain
wolfenrain
reviewed
May 31, 2022
| position: position, | ||
| onPressed: onPressed, | ||
| onReleased: onReleased, |
There was a problem hiding this comment.
Shouldnt this be part of a separate PR? For the sake of clear commits
There was a problem hiding this comment.
It was the other way around really, I wanted the button components to be the same and to do that I introduced the mixin, I can change the title to reflect on that.
spydon
changed the title
HudButtonComponent constructor and ViewportMargin mixin to avoid code duplication
erickzanardo
approved these changes
Jun 1, 2022
| /// If you set the position of the component instead of a margin when | ||
| /// initializing the component, the margin to the edge of the screen from that | ||
| /// position will be used. | ||
| mixin ComponentViewportMargin on PositionComponent, HasGameRef { |
There was a problem hiding this comment.
Idk if hud button or button component have tests, but if they don't this would be a good opportunity to introduce a test suite that would end up testing both things.
spydon
force-pushed
the
spydon.viewport-margin
branch
from
37ea2a4
to
85f3bb8
Compare
wolfenrain
approved these changes
Jun 4, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Created a mixin to handle the margin to the viewport, mostly to remove the code duplication in
ButtonComponentandHudButtonComponent, but it can also be useful in other cases when you don't want to wrap your component inHudMarginComponent.Once the new camera system is in place this will of course have to be replaced.
Checklist
fix:,feat:,docs:etc).docsand added dartdoc comments with///.examples.Breaking Change
Related Issues