-
Notifications
You must be signed in to change notification settings - Fork 1
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
perf: refactor header generation #96
Conversation
Reviewer's Guide by SourceryThe PR refactors the header generation in the AuroTable component by extracting it into a separate method. Instead of using lit-html template literals for header creation, the implementation now uses direct DOM manipulation for better performance. The header generation is triggered when the columnHeaders property changes. Sequence diagram for header generation in AuroTablesequenceDiagram
participant User
participant AuroTable
participant DOM
User->>AuroTable: Update columnHeaders
AuroTable->>AuroTable: updated(changedProperties)
AuroTable->>AuroTable: extractHeaders()
AuroTable->>DOM: Clear existing headers
AuroTable->>DOM: Create and append new headers
Updated class diagram for AuroTable componentclassDiagram
class AuroTable {
+updated(changedProperties)
+firstUpdated()
-extractHeaders()
-extractRows(columns, data)
columnHeaders
componentData
}
note for AuroTable "extractHeaders method added for header generation"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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 @jason-capsule42 - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider reverting to the original template literal implementation for header generation. Direct DOM manipulation with createElement is less idiomatic in lit-element and potentially less performant than letting lit-element handle the rendering through its template system.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 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.
2b751e1
to
ffdab8d
Compare
@sourcery-ai review |
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 @jason-capsule42 - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- Use textContent instead of innerHTML for better security and performance (link)
Overall Comments:
- Consider reverting to using Lit's template literal syntax (html``) instead of direct DOM manipulation. This would be more idiomatic, potentially more performant due to Lit's built-in optimizations, and easier to maintain.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🔴 Security: 1 blocking issue
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 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.
🎉 This PR is included in version 3.1.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Alaska Airlines Pull Request
Type of change:
Please delete options that are not relevant.
Checklist:
By submitting this Pull Request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Pull Requests will be evaluated by their quality of update and whether it is consistent with the goals and values of this project. Any submission is to be considered a conversation between the submitter and the maintainers of this project and may require changes to your submission.
Thank you for your submission!
-- Auro Design System Team
Summary by Sourcery
Enhancements: