-
Notifications
You must be signed in to change notification settings - Fork 831
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
[EuiComboBox] Update to dogfood EuiTextTruncate
#7028
Merged
Merged
Changes from 5 commits
Commits
Show all changes
49 commits
Select commit
Hold shift + click to select a range
23aba4c
:lipstick: Move end truncation in separate class
dej611 00720dc
:heavy_plus_sign: Add canvas mock library for jest
dej611 8b67204
:sparkles: Add truncation middle feature
dej611 00241ea
:memo: Update documentation
dej611 df85489
Merge remote-tracking branch 'origin/master' into fix/5997
dej611 83bd86a
:ok_hand: Remove canvas usage + remove dependency
dej611 ea1be6c
:fire: Remove ellipsis
dej611 cb37976
:recycle: Reduce append operations
dej611 e269b00
Merge branch 'main' into fix/5997
cee-chen 1aeec8c
Revert yarn.lock changes
cee-chen e07791d
revert jest config file changes
cee-chen 6131706
Set up `EuiTextTruncate` component
cee-chen ce0bf7e
export / rest spread
cee-chen 0261f23
Merge branch 'main' into fix/5997
cee-chen 79cb351
More component cleanup/changes
cee-chen 3f9671f
Add `onResize` callback + resize observer story
cee-chen d5d6f3d
Add startEnd anchor position logic needed for combobox search
cee-chen 006eefa
[DX] Make the render prop optional
cee-chen 0c0703c
[typing] Make `truncation` prop optional
cee-chen 572ce0c
Rename `startEndAnchor` to `truncationPosition` + other cleanup
cee-chen 09cc1c3
Write tests
cee-chen b45b719
Add docs page and examples for new component
cee-chen 17047d1
Fix/workaround two crashing `truncationOffset` loops
cee-chen 1af3b49
Improve screen reader and copy UX
cee-chen f46b348
Revert EuiComboBox changes to main
cee-chen 8491224
Add new `truncationProps` and `renderTruncatedOption` logic
cee-chen c69441a
Add docs section for combobox truncation
cee-chen 10dc0e2
Attempt to fix Cypress CI failures caused by fonts
cee-chen f219119
[REFACTOR] Move truncation string logic to a separate vanilla JS class
cee-chen 64aaebd
Clean up `truncationOffset` edge case behavior
cee-chen 7d5ce5a
Fix storybook controls
cee-chen 5fd309e
it's hard to word good
cee-chen a5837bb
[PR feedback] Expand utilities for to specify DOM vs. canvas render m…
cee-chen 767a44e
[misc cleanup] fix `truncateStart` typing
cee-chen 4cd6ad5
Naming things is hard
cee-chen 85ccc1c
Fix CI font issues again :|
cee-chen a1c487d
[PR feedback] Improve perf of DOM measurement
cee-chen b6b67b0
Update `EuiTextTruncate` to support using `TruncationUtilsForCanvas`
cee-chen f4a9af4
Add documentation section and warnings around performance
cee-chen 4a96de8
[misc cleanup] improve tests
cee-chen 879b38e
Add CSS comments/dev documentation
cee-chen 30196af
Minor test cleanup
cee-chen 6718b37
Merge branch 'main' into fix/5997
cee-chen 5098c4c
changelog
cee-chen 3cb8c0d
Make docs example less confusing
cee-chen e28a66e
Merge branch 'main' into fix/5997
cee-chen 998ecb1
Add tests
cee-chen d188bb5
[Cypress] Fix font calculation
cee-chen b971297
Add `shouldRenderCustomStyles` Jest tests
cee-chen File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
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.
I would personally lean towards avoiding the canvas API for this unless there's a strong reason to do so. My reasoning against is that it avoids an extra dependency and also because it's not something the EUI team is familiar with, and thus would be more difficult for us to maintain than just vanilla JS.
If canvas is absolutely the way to go, I'd then strongly prefer to use Cypress to test the functionality over adding a Jest dependency.
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.
I can change the code to not use Canvas API. I think same can be achieved using a
div
element as well.