Error text overflow #2432
Draft
Error text overflow #2432
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@m-akinc, will you buddy this PR for me? |
m-akinc
approved these changes
Oct 9, 2024
There was a problem hiding this comment.
Could/should we replace the ErrorPattern interface with a mixin? Would be nice to remove all the declarations of those three properties and automatically enforce the @attr and @observable expectations.
| <div | ||
| class="error-text" | ||
| ${overflow('errorHasOverflow')} | ||
| title="${x => (x.errorHasOverflow && x.errorText ? x.errorText : null)}" |
There was a problem hiding this comment.
Probably worthwhile to write unit tests for this behavior against one (not all) of the components that use this pattern. I'm pretty sure we have some existing title overflow behavior tests for labels, etc.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request
🤨 Rationale
Fixes #2431
👩💻 Implementation
The shared error pattern has been updated to use the
overflowdirective to track when the error text has overflow and to only show thetitlewhen overflow is present. As part of this, each component that implementsErrorPatternwas updated to include anobservableproperty namederrorHasOverflowto correctly implement theErrorPatterninterface.🧪 Testing
Manually tested in storybook that all updated components:
titleon the error text when the error text is shorttitleon the error text when the error text overflows✅ Checklist