Skip to content
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

fix: control show less/more button at public profile bio #486

Merged
merged 6 commits into from
Feb 8, 2024

Conversation

LinaYahya
Copy link
Contributor

@LinaYahya LinaYahya commented Jan 11, 2024

closes #480

@LinaYahya LinaYahya self-assigned this Jan 11, 2024
@pyphilia
Copy link
Contributor

I'm not really convinced by the line-height-ref method. Plus there're some mismatching problem here:

  • The member profile does not show html normally, but you use the content description that uses interweave that might render html. But your implementation handles text lines only.
  • on the other side we have also a pb with content description because it does not show html if not expanded In collection view show rendered description on "show less" #466

I think a low-level component should include the button that expand button + content with fading (mainly the issue I've linked). Maybe it should be adaptable to use or not interweave. What do you think @spaenleh ? So this PR would handle both issues.

@spaenleh spaenleh force-pushed the 480-bio-show-less-more branch from 24fa1cd to f25bfb3 Compare January 15, 2024 09:30
@spaenleh
Copy link
Member

spaenleh commented Jan 15, 2024

@pyphilia @LinaYahya I agree. Also if the computation actually just returns the line height, why not simply use the lh css unit ?
I think component wise we should have:

  • a DescriptionWrapper that receives a child (the description) which is rendered with a fixed height (2 or 3 lh) when showing less and un-constrained height when showing more + the button to do so.
  • one or multiple Description components (one with Interweave for html descriptions, one simple text for the bio description)

Then you compose them together to get the desired result

@LinaYahya LinaYahya force-pushed the 480-bio-show-less-more branch from 03ac9f0 to 33ff370 Compare February 8, 2024 08:48
@LinaYahya LinaYahya force-pushed the 480-bio-show-less-more branch from 33ff370 to b1cfea1 Compare February 8, 2024 08:52
Copy link

sonarqubecloud bot commented Feb 8, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@spaenleh spaenleh merged commit 6d542d2 into main Feb 8, 2024
3 checks passed
@spaenleh spaenleh deleted the 480-bio-show-less-more branch February 8, 2024 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Public Member Page: do not show "show more show less" if there's no description
3 participants