-
Notifications
You must be signed in to change notification settings - Fork 34
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
Add show more text component #3736
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
import React from 'react'; | ||
|
||
import ReadMore from '@fawazahmed/react-native-read-more'; | ||
|
||
import {COLORS} from '../../../../../shared/src/constants/colors'; | ||
|
||
import {useTranslation} from 'react-i18next'; | ||
|
||
import {Body16} from '../Typography/Body/Body'; | ||
|
||
const readMoreTextStyles = {color: COLORS.PRIMARY}; | ||
|
||
const ShowMoreText: React.FC<{ | ||
numberOfLines?: number; | ||
children: React.ReactNode; | ||
}> = ({numberOfLines = 5, children}) => { | ||
const {t} = useTranslation('Component.ShowMoreText'); | ||
|
||
return ( | ||
<ReadMore | ||
customTextComponent={Body16} | ||
numberOfLines={numberOfLines} | ||
seeLessText={t('shrinkLabel')} | ||
seeMoreText={t('expandLabel')} | ||
seeMoreStyle={readMoreTextStyles} | ||
seeLessStyle={readMoreTextStyles} | ||
animate={false}> | ||
{children} | ||
</ReadMore> | ||
); | ||
}; | ||
|
||
export default ShowMoreText; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,7 +28,6 @@ import {ModalStackProps} from '../../../lib/navigation/constants/routes'; | |
import useExerciseById from '../../../lib/content/hooks/useExerciseById'; | ||
import {formatContentName} from '../../../lib/utils/string'; | ||
import {COLORS} from '../../../../../shared/src/constants/colors'; | ||
import Markdown from '../../../lib/components/Typography/Markdown/Markdown'; | ||
import Byline from '../../../lib/components/Bylines/Byline'; | ||
import {Body14} from '../../../lib/components/Typography/Body/Body'; | ||
import Badge from '../../../lib/components/Badge/Badge'; | ||
|
@@ -59,6 +58,7 @@ import FeedbackCard from '../../../lib/components/FeedbackCard/FeedbackCard'; | |
import ExerciseCardContainer from '../../../lib/components/Cards/SessionCard/ExerciseCardContainer'; | ||
import useExercisesByTags from '../../../lib/content/hooks/useExercisesByTags'; | ||
import {Tag as TagType} from '../../../../../shared/src/types/generated/Tag'; | ||
import ShowMoreText from '../../../lib/components/ShowMoreText/ShowMoreText'; | ||
|
||
const Content = styled(Gutters)({ | ||
justifyContent: 'space-between', | ||
|
@@ -259,7 +259,7 @@ const CompletedSessionModal = () => { | |
<> | ||
<Spacer16 /> | ||
<Gutters> | ||
<Markdown>{exercise?.description}</Markdown> | ||
<ShowMoreText>{exercise?.description}</ShowMoreText> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One tiny thing. Aren't we loosing the possibility to use markdown with this? Wrap it in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Markdown doesn't seem to support There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah you mean using the library, I need to try that out |
||
</Gutters> | ||
</> | ||
)} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
{ | ||
"en": { | ||
"expandLabel": "show more", | ||
"shrinkLabel": "show less" | ||
}, | ||
"pt": {}, | ||
"sv": {}, | ||
"es": {} | ||
} |
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.
Could you explain the benefits of this lib? Couldn't we just do this with a character count and simple state in this component? The lib is a bit sloppy built (e.g. hard to find the source code) so I'm a bit hesitant at adding it.
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.
aha, I approached it similarly (mindset wise) using
But it didn't work well at all, would be interesting trying with character count as you mentioned
(also couldn't add the show more button by the end of the last line, but that's not a required thing)
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.
Seems like counting characters wouldn't work without a custom parser, because we would need to count
\n\n` differently, since it fills more space, as the following:

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.
on the example above I'm using
numberOfLines={5}
on the text to limitThere 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.
Expo seems to have a lib for this as well https://github.com/expo/react-native-read-more-text
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.
They seem to use an api to find out the height of the text component
https://github.com/expo/react-native-read-more-text/blob/3e63ca30bd0b7925893cf76bc015ab7e9b7a75a3/index.js#L103-L105
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.
Shall I copy their approach and do a component in our codebase? wdyt? @gewfy
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.
Thanks for doing some research here! I believe we did something similar, height based, in the old. Let me check :)
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.
Yeah exactly, in the old ap we did what you proposed ⬆️
And then just absolute position a
Show more
at the right bottom of that component, or bellow.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.
Since this seems more complex than it looks on the surface I leave it up to you. Fine to go with the lib you chose too :)