-
-
Notifications
You must be signed in to change notification settings - Fork 347
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
feat: Collection description long text wrap #5383
Conversation
SUCCESS @Jarsen136 PR for issue #5374 which is assigned to you. Please wait for review and don't hesitate to grab another issue in the meantime! |
✅ Deploy Preview for koda-nuxt ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
OK, It would show more words now.
I have no idea to make it at the end of the sentence now. Because we also have a markdown format of the description, I have to put the see more button under the description section.
Underline was added. |
@Jarsen136 nice! last changes
|
That's OK. Will check later |
Fixed |
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.
Otherwise good
|
||
const toggleSeeAllDescription = () => { | ||
seeAllDescription.value = !seeAllDescription.value | ||
} | ||
|
||
const hasSeeAllDescriptionOption = computed(() => { | ||
return ( | ||
(collectionInfo.value?.meta?.description?.length || 0) > | ||
DESCRIPTION_MAX_LENGTH | ||
) | ||
}) | ||
|
||
const visibleDescription = computed(() => { | ||
const desc = collectionInfo.value?.meta?.description | ||
|
||
return ( | ||
(!hasSeeAllDescriptionOption.value || seeAllDescription.value | ||
? desc | ||
: desc?.slice(0, DESCRIPTION_MAX_LENGTH) | ||
)?.replaceAll('\n', ' \n') || '' | ||
) | ||
}) | ||
|
||
const { stats } = useCollectionDetails({ collectionId: collectionId.value }) |
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.
, Would move
It into the 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.
50 via max Len prop it would se available else where
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.
Do you mean moving these parts into markdown component? If so, PR #5366 should be merged firs, then we could do the refactor.
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.
Lets do it in other PR
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.
DescriptionWrapper
is sitting in the corner :(
Not confiant about this "link" button variant, I would replace it with stylised <a />
or create is-text equivalent
Thanks for reminding me of it. I think we are about to remove this component after the recent collection redesign as it does not meet our requirements anymore. I will create a new markdown wrapper component.
Good idea. I have wrapped it with |
@Jarsen136 please fix deepscan and we can continue |
Code Climate has analyzed commit e0db4c1 and detected 0 issues on this pull request. View more on Code Climate. |
It's fixed after merging main branch. |
pay 20 usd |
pay 20 usd |
1 similar comment
pay 20 usd |
😍 Perfect, I’ve sent the payout 🪅 Let’s grab another issue and get rewarded! |
Thank you for your contribution to the KodaDot NFT gallery.
👇 _ Let's make a quick check before the contribution.
PR Type
Context
Before submitting pull request, please make sure:
Optional
Had issue bounty label?
Community participation
Screenshot 📸
As we also have a markdown format of the description, I have to put the see more button under the description section.