-
Notifications
You must be signed in to change notification settings - Fork 56
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
fix: Added contentdxOffset #80
Conversation
WalkthroughThe updates involve adding a new parameter Changes
Poem
TipsChat with CodeRabbit Bot (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- lib/src/popover.dart (3 hunks)
- lib/src/popover_item.dart (3 hunks)
Additional comments: 7
lib/src/popover.dart (4)
56-58: The addition of the
contentDxOffset
parameter with a default value of 0 is correctly implemented and follows the pattern of other offset parameters in the function.139-139: The
contentDxOffset
parameter is correctly passed to thePopoverItem
constructor, ensuring that the new feature is integrated into the popover's layout logic.56-58: The documentation for the
contentDxOffset
parameter is clear and consistent with the existing documentation style for other parameters.95-95: The default value of 0 for
contentDxOffset
is a safe choice as it maintains the current behavior when the parameter is not explicitly set by the user.lib/src/popover_item.dart (3)
144-148: The implementation of
contentDxOffset
in_configureRect
looks correct and should allow for the intended horizontal offset adjustment of the popover content.37-40: The default values for
contentDxOffset
and other offset properties are set to 0, which is good for maintaining backward compatibility.144-148: Ensure that the
arrowWidth
property is always non-null when it's being used in the_configureConstraints
method, as it's added tomaxWidth
without a null check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (1)
- lib/src/popover.dart (4 hunks)
Additional comments: 3
lib/src/popover.dart (3)
56-58: The addition of the
contentDxOffset
parameter to theshowPopover
function is correctly implemented with a default value of 0, which is a safe default that maintains backward compatibility.122-122: The
PopScope
widget is not a standard Flutter widget. Verify if it is a custom widget and ensure it is correctly implemented and tested.85-101: The logic to handle
constraints
based onwidth
andheight
is correctly implemented, ensuring that the popover respects the provided dimensions if any.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- lib/src/popover.dart (4 hunks)
Additional comments: 4
lib/src/popover.dart (4)
119-123: Verify the implementation of
PopScope
as it is not a standard Flutter widget. Ensure that it properly handles the on-pop callback and that it is tested for the intended behavior.98-98: The
contentDxOffset
parameter is correctly added with a default value of 0, which is consistent with the summary and the documentation.146-146: The
barrierLabel
is correctly handled with a fallback toMaterialLocalizations.of(context).modalBarrierDismissLabel
when it isnull
.149-149: The
transitionBuilder
is correctly handled with a fallback to a defaultFadeTransition
whennull
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- lib/src/popover.dart (4 hunks)
Additional comments: 2
lib/src/popover.dart (2)
53-62: The addition of
contentDxOffset
to theshowPopover
function is consistent with the summary and allows for horizontal positioning of the popover content. Ensure that thePopoverItem
class is updated accordingly to handle this new parameter.136-148: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [119-148]
The replacement of
WillPopScope
withPopScope
for handling popover dismissal behavior is consistent with the summary. Verify thatPopScope
is a valid widget and that theonPopInvoked
callback is correctly implemented to handle the dismissal.
@kasyanyukd1995 let's bump SDK and Flutter versions to the latest stable, please: environment:
sdk: '>=3.2.1 <4.0.0'
flutter: '>=3.16.1' |
@kasyanyukd1995 thank you for your contribution! I'll cut a new release tomorrow. |
I added contentDxOffset
Summary by CodeRabbit
New Features
contentDxOffset
parameter.Enhancements
PopScope
instead of `WillPopScope.Refactor