Set max text height to prevent overflow

[kdumontnu]

Sets a max height for review text boxes to prevent a very annoying bug where users cannot access the "submit" button.

Before:

After:

Interestingly, I don't see this bug on Firefox.

[silverwind]

I think this is an issue with the overall page layout where something prevents this dropdown from creating scrollable page overflow.

That said, this fix does not seem to work in all cases. I still see dropdown cut off on small vertical viewport.

Also I found a potentially related comment here:

gitea/web_src/js/features/repo-issue.js

Lines 463 to 465 in 2b5e013

	// the editor's height is too large in some cases, and the panel cannot be scrolled with page now because there is `.repository .diff-detail-box.sticky { position: sticky; }`
	// the temporary solution is to make the editor's height smaller (about 4 lines). GitHub also only show 4 lines for default. We can improve the UI (including Dropzone area) in future
	await createCommentEasyMDE($reviewBox.find('textarea'), {minHeight: '80px'});

.

[wxiaoguang]

1. EasyMDE has it's own options, see https://github.com/Ionaru/easy-markdown-editor , maxHeight , and our code already supports the customized options, grep createCommentEasyMDE(textarea, easyMDEOptions = {})
2. calc(100vh - 920px) is incorrect. What if 100vh=930px, then minaxHeight=10px? , if we set minHeight and maxHeight together, then minHeight works if maxHeight is less.

[silverwind]

We should not need to deal with max-height at all. Instead we should allow the page to overflow in such a case. If content goes below the footer, that's okay, I see it happening on GitHub too.

[wxiaoguang]

GitHub's behavior is indeed better.

At that time, minHeight was enough for most cases, and .diff-detail-box.sticky { position: sticky; } makes the page difficult to be overflowed with scrollbar, so I just left a comment there (I hadn't looked into it carefully, maybe I was wrong).

Although there might need some more CSS work, making the page can be overflowed correctly could be better.

Either is fine to me and I can also accept that we use maxHeight here as a temporary solution as this PR.

[silverwind]

position: sticky alone does not prevent overflow. There's something else preventing it, but it's at least no obvious to me.

[kdumontnu]

That said, this fix does not seem to work in all cases. I still see dropdown cut off on small vertical viewport.

Mmm, I see what you mean. I can see it cut off with client height < 400px, which is pretty tiny. Still, I'm not sure how to fix the innate overflow issue and I think this is significantly improved.

[kdumontnu]

Looks like GitHub is broken too, so we can claim feature parity! GitHub breaks at ~450px height, so I call that a win 😄

[zeripath]

Please send backport.

[CirnoT]

Was it the intention of this PR to create this situation?

It would seem that this doesn't apply max-height but min-height instead:

[wxiaoguang]

It seems that EasyMDE can not handle minHeight and maxHeight together correctly, and no one found this problem during code review.

I think we had better revert this PR and propose a new proper one.

[wxiaoguang]

The PR to fix it:

• Fix the editor height in review box #19003