-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: add menu #28 #34
Conversation
Reviewer's Guide by SourceryThis PR adds the auro-menu component to the project, including its implementation, build scripts, documentation, and tests. The menu component provides users with a list of selectable options and supports features like nested menus, disabled states, keyboard navigation, and custom events. Class diagram for the new auro-menu componentclassDiagram
class AuroMenu {
-String value
-Object optionSelected
-String matchWord
-Boolean noCheckmark
-Boolean ready
-Object optionActive
-Boolean rootMenu
-AuroLibraryRuntimeUtils runtimeUtils
-String nestingSpacer
+resetOptionsStates()
+makeSelection()
+selectNextItem(String moveDirection)
+selectByValue(String value)
+updateActiveOption(int index)
+handleSlotItems()
+notifyReady()
}
class AuroMenuOption {
-Boolean nocheckmark
-Boolean disabled
-Boolean selected
-String value
-Number tabIndex
-AuroLibraryRuntimeUtils runtimeUtils
+generateIconHtml(String svgContent)
}
AuroMenu --> AuroMenuOption : uses
AuroMenuOption --> AuroIcon : uses
AuroMenuOption --> AuroLibraryRuntimeUtils : uses
AuroMenu --> AuroLibraryRuntimeUtils : uses
AuroMenu --> AuroMenuOption : contains
AuroMenuOption --> AuroIcon : contains
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
0b16e4c
to
11ac6c1
Compare
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 @sun-mota - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider documenting the rationale for changing
this.value = undefined
tothis.value = ""
in base-input.js, as this could affect other components using this base class. - The build scripts contain some duplication for menu-specific tasks. Consider refactoring to make the build configuration more DRY while maintaining explicit behavior.
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟢 Security: all looks good
- 🟡 Testing: 2 issues found
- 🟡 Complexity: 2 issues 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.
f55244e
to
f59fbf9
Compare
@@ -9,10 +9,10 @@ export default { | |||
}, | |||
coverageConfig: { | |||
threshold: { | |||
statements: 80, | |||
statements: 70, |
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.
This is fine for now, but we need to have more of a conversation about test coverage. We should be adding more tests to cover code rather than lowering the thresholds. By default, all components should be at least at 80%.
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.
Maybe open a ticket for this?
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.
this change is temporary workaround, recommended by Jason. The ultimate fix will be needed, ticketed at #47
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.
Run npm run build:release
from root level of form to address updates to min.js
file
424e919
to
2ebe393
Compare
Alaska Airlines Pull Request
close #28
adding
auro-menu@4.1.4
toauro-formkit
By submitting this Pull Request, I confirm that my contribution is made under the terms of the Apache 2.0 license and I have performed a self-review of my own update.
Summary by Sourcery
Add a new 'auro-menu' component to the project, complete with build scripts, documentation, and tests to support its integration and usage.
New Features:
Enhancements:
Documentation:
Tests: