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 input line-height cutting off g #25334

Merged
merged 7 commits into from
Jun 27, 2023
Merged

Fix input line-height cutting off g #25334

merged 7 commits into from
Jun 27, 2023

Conversation

hiifong
Copy link
Member

@hiifong hiifong commented Jun 18, 2023

Fix the incomplete display of input text
Before:
image
image
After:
image
image

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jun 18, 2023
@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jun 18, 2023
@silverwind
Copy link
Member

I think there ought to be better solutions, but I can not reproduce the issue. Which browser/os do you see this on?

Here is Firefox Mac:

image

@hiifong
Copy link
Member Author

hiifong commented Jun 18, 2023

I think there ought to be better solutions, but I can not reproduce the issue. Which browser/os do you see this on?

Here is Firefox Mac:

image

Google Chrome Windows11

@hiifong
Copy link
Member Author

hiifong commented Jun 18, 2023

I think there ought to be better solutions, but I can not reproduce the issue. Which browser/os do you see this on?

Here is Firefox Mac:

image

image
Microsoft Edge

@silverwind
Copy link
Member

Chrome on Mac seems fine as well:

image

@hiifong
Copy link
Member Author

hiifong commented Jun 18, 2023

Chrome on Mac seems fine as well:

image

Chrome on ubuntu is also
image

@silverwind
Copy link
Member

Can you check if there is any overflow: hidden style maybe where you reproduce it? I thought the default behaviour of inputs is that they don't cut off their value like that. I really wonder whether this is a Blink bug if there is no overflow: hidden style anywhere.

@@ -279,6 +279,10 @@ button {
cursor: pointer;
}

input {
line-height: 100% !important;
Copy link
Contributor

Choose a reason for hiding this comment

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

It can't be right if you put a "!important" on a general element/style. It would pollute uncontrollable styles and cause unfixable overriding bugs.

Copy link
Member Author

Choose a reason for hiding this comment

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

What about replacing element selectors with class selectors?

Copy link
Member

Choose a reason for hiding this comment

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

Before looking for such workarounds, check why the problem is happening in first place. Does a unstyled <input> element reproduce the problem? If not, add the same CSS properties we do until it does show the issue, then you have the culprit.

Copy link
Member Author

Choose a reason for hiding this comment

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

I will test it

Copy link
Member

@silverwind silverwind Jun 19, 2023

Choose a reason for hiding this comment

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

Likely the problematic style is

.ui.input > input {
  line-height: 1.21428571em;
}

Can you reproduce the problem on https://fomantic-ui.com/collections/form.html#form? If you, we can override this style to maybe 1.25em, just enough to fix your issue basically.

If you wanna be completely thorough, also report a bug upstream to https://github.com/fomantic/Fomantic-UI, but keep in mind we're not going to update fomantic-ui anymore as we are trying to migrate off of it currently.

Copy link
Member

Choose a reason for hiding this comment

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

Scroll down there, it should be somewhere.

Copy link
Member Author

Choose a reason for hiding this comment

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

image

Copy link
Member Author

Choose a reason for hiding this comment

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

is here?
image

Copy link
Member

@silverwind silverwind Jun 20, 2023

Choose a reason for hiding this comment

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

It should be somewhere, otherwise the CSS your browser has received is incomplete. Can you check the Computed tab for the line-height value?

In any case, I'll probably see what I can do and push a fix here.

Copy link
Member Author

Choose a reason for hiding this comment

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

image

@hiifong
Copy link
Member Author

hiifong commented Jun 22, 2023

When I select the text, the text is displayed fine.

Windows11 Chrome:

1.mp4

Ubuntu20.04 Chrome:

2.mp4

@silverwind
Copy link
Member

So I reproduced this on Windows Chrome. It is related to our choice of fonts apparently. I've pushed a fix that overrides the problematic Fomantic CSS without !important for the least possible impact.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jun 26, 2023
@silverwind silverwind changed the title set input line-height to 100% Fix input line-height cutting off g Jun 26, 2023
@silverwind silverwind added the backport/v1.20 This PR should be backported to Gitea 1.20 label Jun 26, 2023
web_src/css/base.css Outdated Show resolved Hide resolved
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jun 27, 2023
web_src/css/base.css Outdated Show resolved Hide resolved
@silverwind silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jun 27, 2023
@silverwind silverwind enabled auto-merge (squash) June 27, 2023 08:34
@silverwind silverwind disabled auto-merge June 27, 2023 08:38
web_src/css/base.css Outdated Show resolved Hide resolved
@silverwind silverwind enabled auto-merge (squash) June 27, 2023 08:39
@silverwind silverwind merged commit 1069472 into go-gitea:main Jun 27, 2023
@GiteaBot GiteaBot added this to the 1.21.0 milestone Jun 27, 2023
@GiteaBot GiteaBot added backport/done All backports for this PR have been created and removed reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. labels Jun 27, 2023
@hiifong hiifong deleted the fix_input_line_height branch July 9, 2023 09:19
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Sep 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created backport/v1.20 This PR should be backported to Gitea 1.20 lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants