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

Have a way to disable max-height on comments #181846

Closed
kazemicode opened this issue May 9, 2023 · 11 comments · Fixed by #185301
Closed

Have a way to disable max-height on comments #181846

kazemicode opened this issue May 9, 2023 · 11 comments · Fixed by #185301
Assignees
Labels
comments Comments Provider/Widget/Panel issues feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@kazemicode
Copy link

PR #180044 introduced a max-height on comments, which goes against my team's use case when using the CodeTour extension. Since our tours have animated GIFs that support our hands-on coding labs, the reduced max-height makes it impossible for a user to see entire images in the comment frame at once, which imposes a negative learner experience. (See: https://www.youtube.com/watch?v=h0F8_-Te8p4 for comparison across versions).

Proposed feature is to allow a user to set a max-height rather than imposing a max-height.

@gjsjohnmurray
Copy link
Contributor

I also think it should be settable through the API. My original commit on the PR has a comment exchange with @alexr00 about this at the place where I originally hardwired it.

@alexr00
Copy link
Member

alexr00 commented May 9, 2023

Generally we try not to add API unless we see a clear case for having it. At the time @gjsjohnmurray suggested the max height I wasn't aware of a use case that would require a comment peek view to have a larger size.

@kazemicode your case makes sense, and makes the case for having API for this. I would make this API rather than user configurable.

@alexr00 alexr00 added feature-request Request for new features or functionality api api-proposal comments Comments Provider/Widget/Panel issues labels May 9, 2023
@alexr00 alexr00 added this to the June 2023 milestone May 9, 2023
@kazemicode
Copy link
Author

kazemicode commented May 9, 2023

Yes, making the max-height configurable via the API makes perfect sense, since any extension can be configured with that functionality to set a max-height attribute that makes sense for the use case.

@alexr00 Would the default behavior go back to expanding the height with the content?

@alexr00
Copy link
Member

alexr00 commented May 9, 2023

@kazemicode I'm not sure yet which would be default. I will discuss with our UX folks and see what they prefer.

@kazemicode
Copy link
Author

@alexr00 My humble preference would be to adopt the old behavior as the default. Though the CodeTour extension is open source, it’s only seen one update in the past year. My team has a stale PR from 2021 on the CodeTour repo that was never commented on, so I’m afraid that even if we opened a PR with a fix for the max-height problem, it might not ever get accepted and merged. On the other hand, my team has been discussing forking CodeTour so that we can control the addition of new functionality and bug fixes.

@kazemicode kazemicode changed the title Allow end user to set max-height on comments instead of imposing a max-height Add API functionality to set max-height on comments instead of imposing a max-height May 9, 2023
@alexr00 alexr00 changed the title Add API functionality to set max-height on comments instead of imposing a max-height Have a way to disable max-height on comments Jun 16, 2023
@alexr00
Copy link
Member

alexr00 commented Jun 16, 2023

There wasn't enough interest in adding API for this, so I added the user setting "comments.maxHeight".

@VSCodeTriageBot VSCodeTriageBot added the unreleased Patch has not yet been released in VS Code Insiders label Jun 16, 2023
@kazemicode
Copy link
Author

@alexr00 thanks! Is comments.maxHeight a boolean? If so, is the default true (maintaining the maxHeight as 20em by default) or false (no maxHeight defined?

@alexr00
Copy link
Member

alexr00 commented Jun 16, 2023

It is a boolean and the default is true, meaning the max height of a comment will be 20em.

@VSCodeTriageBot VSCodeTriageBot added insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Jun 16, 2023
@alexr00 alexr00 added verification-needed Verification of issue is requested author-verification-requested Issues potentially verifiable by issue author labels Jun 26, 2023
@VSCodeTriageBot
Copy link
Collaborator

This bug has been fixed in the latest release of VS Code Insiders!

@kazemicode, you can help us out by commenting /verified if things are now working as expected.

If things still don't seem right, please ensure you're on version b4952d1 of Insiders (today's or later - you can use Help: About in the command palette to check), and leave a comment letting us know what isn't working as expected.

Happy Coding!

@kazemicode
Copy link
Author

kazemicode commented Jun 26, 2023

/verified

Works great! Thanks!

@VSCodeTriageBot VSCodeTriageBot added verified Verification succeeded and removed author-verification-requested Issues potentially verifiable by issue author labels Jun 26, 2023
@alexr00
Copy link
Member

alexr00 commented Jun 27, 2023

Thanks for verifying!

@github-actions github-actions bot locked and limited conversation to collaborators Jul 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
comments Comments Provider/Widget/Panel issues feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants