-
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
refactor: remove ready
state and *-ready
event
#74
Conversation
Reviewer's Guide by SourceryThis pull request removes the 'ready' state and associated 'ready' events from multiple components to simplify the initialization process. The changes eliminate the need for components to wait for a ready state before performing certain operations, making the component lifecycle more straightforward. Class diagram for AuroCombobox after removing 'ready' stateclassDiagram
class AuroCombobox {
-menuWrapper
-dropdown
-auroDropdownReady
-auroMenuReady
-auroInputReady
-readyRetryCount
-options
-menu
-input
-value
-optionSelected
-optionActive
-checkmark
-noFilter
-showBib()
-configureInput()
-handleMenuOptions()
-handleInputValueChange()
-configureDropdown()
-configureCombobox()
-handleSlotChange()
-updated(changedProperties)
}
note for AuroCombobox "Removed 'ready' state and related methods"
Class diagram for AuroSelect after removing 'ready' stateclassDiagram
class AuroSelect {
-dropdown
-auroDropdownReady
-auroMenuReady
-readyRetryCount
-menu
-value
-optionActive
-validation
-configureDropdown()
-configureSelect()
-generateOptionsArray()
-handleMenuOptions()
-handleInputValueChange()
-updated(changedProperties)
}
note for AuroSelect "Removed 'ready' state and related methods"
Class diagram for AuroDropdown after removing 'ready' stateclassDiagram
class AuroDropdown {
-inset
-placement
-rounded
-tabIndex
-noToggle
-bibContent
-floater
-handleTriggerTabIndex()
}
note for AuroDropdown "Removed 'ready' state and related methods"
Class diagram for AuroMenu after removing 'ready' stateclassDiagram
class AuroMenu {
-optionSelected
-matchWord
-noCheckmark
-optionActive
-value
-items
-index
-handleSlotItems()
-updateActiveOption(index)
}
note for AuroMenu "Removed 'ready' state and related methods"
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
f7ab282
to
fba3ec3
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 and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟡 Testing: 2 issues found
- 🟢 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.
@@ -84,7 +84,6 @@ describe('auro-combobox', () => { | |||
|
|||
it('hides the bib when making a selection', async () => { | |||
const el = await defaultFixture(); | |||
await waitUntil(() => el.ready); | |||
|
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.
suggestion (testing): Remove unnecessary ready state checks from tests
Since the ready state and events have been removed, we should remove these waitUntil checks. The component initialization is now synchronous so we can proceed with the test assertions immediately after fixture creation.
@@ -104,7 +104,6 @@ describe('auro-select', () => { | |||
|
|||
it('make invalid selection programmatically results in error ui', async () => { | |||
const el = await presetValueFixture(); | |||
await waitUntil(() => el.ready); | |||
|
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.
suggestion (testing): Remove ready state checks from select component tests
Similar to the combobox tests, we should remove these ready state checks since the initialization is now synchronous. This will simplify the tests and make them more straightforward.
it('hides the bib when making a selection', async () => {
const el = await defaultFixture();
const trigger = el.dropdown.querySelector('[slot="trigger"]');
fba3ec3
to
c5533d4
Compare
🎉 This PR is included in version 1.6.0-beta.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Alaska Airlines Pull Request
closed #72
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
Refactor components to remove the 'ready' state and 'ready' events, simplifying the initialization process and updating documentation and tests accordingly.
Enhancements:
Documentation:
Tests: