-
Notifications
You must be signed in to change notification settings - Fork 31
feat: add instance.get_classes for template components
#268
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
Conversation
Reviewer's Guide by SourceryThis pull request introduces a Sequence diagram for get_attributes methodsequenceDiagram
participant Instance as FrontendComponent Instance
Instance->>Instance: get_attributes()
Instance->>Instance: get_classes()
Instance->>Instance: config.get("attributes", {})
Instance->>Instance: additional_classes
Instance->>Instance: conditional_escape(" ".join(classes))
Instance-->>Instance: return classes
Instance->>Instance: f'class="{classes}"'
Instance->>Instance: attributes.items()
Instance->>Instance: f'{item}="{conditional_escape(value)}"'
Instance->>Instance: (classes + " " + parts).strip()
Instance->>Instance: mark_safe(" " + attributes_string)
Instance-->>Instance: return attributes_string
Updated class diagram for FrontendComponentclassDiagram
class FrontendComponent {
+config
+additional_classes
+add_attribute(attr, value)
+get_attributes()
+get_classes()
+save(*args, **kwargs)
}
note for FrontendComponent.get_classes "New method to get classes from attributes and additional classes"
note for FrontendComponent.get_attributes "Refactored to use get_classes method"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #268 +/- ##
==========================================
+ Coverage 88.64% 88.65% +0.01%
==========================================
Files 124 124
Lines 3363 3367 +4
Branches 284 284
==========================================
+ Hits 2981 2985 +4
Misses 264 264
Partials 118 118 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Hey @fsbraun - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a test case that specifically targets the
get_classesmethod to ensure it functions as expected with different mixin combinations. - The change in
component_base.pyreverses the order of mixins; confirm this is intentional and doesn't break existing functionality.
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
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
instance.get_classessimplifies using mixins for spacing, background, etc. in template componentsSummary by Sourcery
Add
get_classesmethod to simplify class management for template components and improve mixin handlingNew Features:
instance.get_classes()method to easily retrieve and combine CSS classes for template componentsEnhancements:
get_classes()methodDocumentation:
get_classes()methodTests: