Skip to content

Conversation

ndonkoHenri
Copy link
Contributor

@ndonkoHenri ndonkoHenri commented Aug 23, 2024

Description

Fixes #3830

Summary by Sourcery

Fix the issue with Snackbar.margin not being effective when set to a Margin object by adjusting the logic to ensure margin is applied only when width is not set.

Bug Fixes:

  • Fix the issue where Snackbar.margin had no effect when set to a Margin object by ensuring margin is correctly applied when width is not set.

Copy link
Contributor

sourcery-ai bot commented Aug 23, 2024

Reviewer's Guide by Sourcery

This pull request addresses an issue where the Snackbar.margin property had no effect when set to a Margin object. The changes implement a fix for this behavior in both the Dart and Python SDKs, ensuring that the margin is properly applied when appropriate.

File-Level Changes

Change Details Files
Modify margin and width handling for SnackBar in Dart
  • Add a condition to set margin to null if width is provided
  • Update comments to clarify behavior
  • Maintain existing logic for non-floating SnackBars
packages/flet/lib/src/controls/snack_bar.dart
Update margin handling in Python SDK
  • Remove conditional setting of margin attribute
  • Always set margin attribute regardless of width
sdk/python/packages/flet-core/src/flet_core/snack_bar.py

Tips
  • Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
  • Continue your discussion with Sourcery by replying directly to review comments.
  • You can change your review settings at any time by accessing your dashboard:
    • Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
    • Change the review language;
  • You can always contact us if you have any questions or feedback.

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @ndonkoHenri - I've reviewed your changes - here's some feedback:

Overall Comments:

  • Please review the changes in the Python file (snack_bar.py). The removal of the condition for setting the margin attribute might need further explanation or reconsideration.
  • Consider adding a brief comment in the Dart file explaining why both margin and width can't be used together, to improve code readability.
Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

@FeodorFitsner FeodorFitsner merged commit 5142f1b into main Aug 27, 2024
3 checks passed
@FeodorFitsner FeodorFitsner deleted the fix-snackbar-margin branch August 27, 2024 04:44
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.

When the margin property of snack_bar is set to ft.Margin, this property has no effect.
2 participants