-
Notifications
You must be signed in to change notification settings - Fork 1
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
[NOREF] Update TruncatedText component to take line clamp prop #1431
base: feature/MINT-1732_echimp-ffs-cr-tdl
Are you sure you want to change the base?
[NOREF] Update TruncatedText component to take line clamp prop #1431
Conversation
const mocks = [ | ||
{ | ||
request: { | ||
query: GetModelPlanBaseDocument, | ||
variables: { id: 'f11eb129-2c80-4080-9440-439cbe1a286f' } | ||
}, | ||
result: { | ||
data: { | ||
modelPlan: {} | ||
} | ||
} | ||
} | ||
]; |
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.
Test throw error if Provider (and therefore mocks) were not included in test. Is there another way to solve this? Currently, this is just a random mock and not used in TruncatedText
component
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.
That's the incorrect query to mock. TruncatedText utilized the MentionTextArea
. We might actually might want to not use this. The MetionTextArea
has a query embedded within it - useSearchOktaUsersLazyQuery
. I forgot why I decided to utilize MentionTextArea
. I don't think that was the right choice, considering how much overhead that component has built in. We should probably try to build TruncatedText
without it. Sorry this was my fault
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 dont think you addressed that unit test in your PR. It's mocking the incorrect query. Also we can replace MentionTextArea if you want, would be a good time. This would make us not need to mock the query
[NOREF]
Description
The
TruncatedText
component currently only takes a character limit. But recently, there have been designs asking to truncate text to xamount of lines (line clamping) instead of character count. This PR updates the
TruncatedText
component to take in line clamp valueHow to test this change
TruncatedText
still renders correctPR Author Checklist
PR Reviewer Guidelines