-
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
fix: use label as placeholder when there is no placeholder string set #36 #157
Conversation
Reviewer's Guide by SourceryThis PR fixes an issue where the placeholder text was not displayed correctly when no placeholder string was set. It changes the default placeholder value to an empty string and uses the label as a placeholder when there is no placeholder or value set. Class diagram for AuroSelect component changesclassDiagram
class AuroSelect {
-String placeholder
-Object optionSelected
-String validity
+constructor()
+render()
}
note for AuroSelect "Changed:
- Default placeholder=''
- Added asPlaceholder attribute
- Modified label slot rendering"
State diagram for select component placeholder behaviorstateDiagram-v2
[*] --> NoPlaceholder: Initial State
NoPlaceholder --> ShowLabel: No placeholder & No value
NoPlaceholder --> ShowValue: Has value
ShowLabel --> ShowValue: Value selected
ShowValue --> ShowLabel: Value cleared
state NoPlaceholder {
[*] --> EmptyString: Default placeholder=''
}
state ShowLabel {
[*] --> UseLabelAsPlaceholder: asPlaceholder=true
}
state ShowValue {
[*] --> DisplaySelectedValue: asPlaceholder=false
}
File-Level Changes
Assessment against linked issues
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 @sun-mota - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding unit tests for the new handleLabelSlotChange functionality to ensure the label-as-placeholder behavior works correctly
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 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.
…ceholder and no value
0a4ca4e
to
665c889
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 @sun-mota - I've reviewed your changes - here's some feedback:
Overall Comments:
⚠️ Changing the default placeholder from 'Please select option' to an empty string is a breaking change that could affect existing applications. Please consider if this needs to be documented in release notes or migration guides.
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.
Alaska Airlines Pull Request
Closes #36
Before Submitting this pull request:
Development
sectionnote: all pull requests require at least one linked ticket
Ready For Review
, all ticket's linked underDevelopment
must have their status changed toReady For Review
as wellBy 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
Bug Fixes: