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

Fixed Read More Button Display Logic to Avoid Unnecessary Display #9627

Closed
wants to merge 6 commits into from

Conversation

CodeMaverick2
Copy link
Contributor

@CodeMaverick2 CodeMaverick2 commented Jul 25, 2024

Closes #9329

Description

This PR addresses an issue where the "Read more" button was displayed even when it made the section longer than the fully expanded content. This led to a confusing user experience, as the "Read more" button was not serving its intended purpose. The following changes have been made to resolve this issue:

Changes

  1. Capture "Read More" Button Height:

    • The height of the "Read more" button is now captured during initialization to ensure accurate calculations.
  2. Update Condition in reset Method:

    • The logic that determines whether the "Read more" button should be displayed has been updated to include the height of the button. This ensures the button is only shown when it makes sense.

Impact

  • Improved User Experience: The "Read more" button is now only displayed when necessary, preventing unnecessary clutter and confusion.
  • Correct Layout Behavior: Ensures the layout behaves as expected, with the "Read more" button enhancing usability rather than detracting from it.

Screenshots

Screenshot from 2024-07-25 18-43-42

@RayBB
Copy link
Collaborator

RayBB commented Jul 31, 2024

@CodeMaverick2 it looks like there is a test that broke because of this change. You'll need to investigate if the test should be changed or if something is actually broken.

@cdrini I am not so familiar with this part of the code and can't really dig into this issue anytime soon. Could we get someone else to review this?

@RayBB RayBB assigned cdrini and unassigned RayBB Jul 31, 2024
Copy link
Collaborator

@cdrini cdrini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @CodeMaverick2 . Please revert the unnecessary changes. And please add testing instruction on how you tested to confirm this change worked. If you cannot test this PR, we will be unable to review your changes.

this.fullHeight = this.$content.scrollHeight;
// Fudge factor to account for non-significant read/more
// (e.g missing a bit of padding)
if (this.$content.scrollHeight <= (this.collapsedHeight + 1)) {
if (this.$content.scrollHeight <= (this.collapsedHeight + this.$readMoreButton.offsetHeight + 1)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be the only change that is necessary here. Please revert the other changes since they are unnecessary.

@cdrini cdrini added the Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] label Aug 30, 2024
@RayBB
Copy link
Collaborator

RayBB commented Sep 11, 2024

@CodeMaverick2 do you think you'll be able to implement Drini's feedback?

@CodeMaverick2
Copy link
Contributor Author

@RayBB Sure i will do that

@github-actions github-actions bot removed the Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] label Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Read more" button should not be added if it takes up as much space as the hidden text
3 participants