Fix Fomantic's line-height causing vertical scrollbars to appear #26961

kerwin612 · 2023-09-07T12:47:14Z

Before:

After:

Remove the scroll bar exception that in the a tag
Reduce the actual width of the a tag to the actual width of the content

As shown in the screenshot, the red box area should not be clickable

web_src/css/base.css

kerwin612 · 2023-09-08T00:34:22Z

For current

For #25597

wxiaoguang

Line-height is the right approach. 👍 (if it doesn't affect others .....)

But I am not sure whether adding it to text truncate is good enough .....

wxiaoguang · 2023-09-08T04:09:32Z

After reading the code again, I think the root problem is that the Fomantic List has some pre-defined "line-height"

/* List Item */
ul.ui.list li,
ol.ui.list li,
.ui.list > .item,
.ui.list .list > .item {
  display: list-item;
  table-layout: fixed;
  list-style-type: @listStyleType;
  list-style-position: @listStylePosition;

  padding: @itemPadding;
  line-height: @itemLineHeight;
}

I believe the line-height fix should be applied to the "li / item" selector.

wxiaoguang · 2023-09-12T06:28:55Z

I guess we can just override the Fomantic UI variable itemLineHeight and build Fomantic again?

https://github.com/go-gitea/gitea/blob/main/web_src/fomantic/_site/globals/site.variables

Then maybe many patched styles could also be removed.

kerwin612 · 2023-09-12T14:47:26Z

I guess we can just override the Fomantic UI variable itemLineHeight and build Fomantic again?

https://github.com/go-gitea/gitea/blob/main/web_src/fomantic/_site/globals/site.variables

Then maybe many patched styles could also be removed.

Personally, I don't think override the Fomantic UI variable is a better solution.

it requires separate compilation
it can affect some internal computational logic in Fomantic. For example, the itemLineHeight variable you mentioned has multiple references within Fomantic, making it difficult to maintain such modifications

So, I think it's easier to maintain it in gitea's own css as needed, but it's also possible that I'm not thinking enough.

wxiaoguang · 2023-09-13T00:47:10Z

I would prefer to change @itemLineHeight variable.

@silverwind what do you think?

kerwin612 · 2023-09-13T01:27:56Z

I would prefer to change @itemLineHeight variable.

@silverwind what do you think?

In fact, I have tried, as I said, because of internal dependencies, resulting in changes will need to do all kinds of compatibility;

Here are some of the dependencies I analyzed:

https://github.com/fomantic/Fomantic-UI/blob/2.8.7/src/definitions/elements/list.less#L61

/* List Item */
ul.ui.list li,
ol.ui.list li,
.ui.list > .item,
.ui.list .list > .item {
    ...
    line-height: @itemLineHeight;
}

https://github.com/fomantic/Fomantic-UI/blob/2.8.7/src/themes/default/elements/list.variables#L20

@itemLineHeight: @relativeLarge;

https://github.com/fomantic/Fomantic-UI/blob/2.8.7/src/themes/default/elements/list.variables#L49

@contentLineHeight: @itemLineHeight;
@contentLineHeightOffset: ((@contentLineHeight - 1em) / 2);

https://github.com/fomantic/Fomantic-UI/blob/2.8.7/src/themes/default/globals/site.variables#L671

@relativeLarge: unit(@largeRaw, em);

This PR focuses on itemLineHeight, We can change itemLineHeight from elements/list.variables or relativeLarge from globals/site.variables, After I try to make only the above changes, the following exception occurs:

Potentially unhandled rejection [2] Error: Error in plugin "gulp-less" 
Message:                                                                 
    Operation on an invalid type in file /kerwin612/.linux/ws/gitea/web_src/fomantic/node_modules/fomantic-ui/src/themes/default/elements/list.variables line no. 48
Details:            
    type: Operation 
    filename: /kerwin612/.linux/ws/gitea/web_src/fomantic/node_modules/fomantic-ui/src/themes/default/elements/list.variables

    index: 1081 
    line: 48 
    column: 15 
    callLine: NaN 
    callExtract: undefined 
,@contentLineHeight: @itemLineHeight;   
    lineNumber: 48
    fileName: /kerwin612/.linux/ws/gitea/web_src/fomantic/node_modules/fomantic-ui/src/themes/default/elements/list.variables 
    at DestroyableTransform.errorHandler (/kerwin612/.linux/ws/gitea/web_src/fomantic/node_modules/fomantic-ui/tasks/config/tasks.js:124:19)  
    at DestroyableTransform.emit (node:events:525:35)  
    at DestroyableTransform.emit (node:domain:489:12) 
    at onwriteError (/kerwin612/.linux/ws/gitea/web_src/fomantic/node_modules/readable-stream/lib/_stream_writable.js:449:12) 
    at onwrite (/kerwin612/.linux/ws/gitea/web_src/fomantic/node_modules/readable-stream/lib/_stream_writable.js:470:11) 
    at WritableState.onwrite (/kerwin612/.linux/ws/gitea/web_src/fomantic/node_modules/readable-stream/lib/_stream_writable.js:180:5) 
    at DestroyableTransform.afterTransform (/kerwin612/.linux/ws/gitea/web_src/fomantic/node_modules/readable-stream/lib/_stream_transform.js:93:3) 
    at /kerwin612/.linux/ws/gitea/web_src/fomantic/node_modules/gulp-less/index.js:58:14 
    at tryCatchReject (/kerwin612/.linux/ws/gitea/web_src/fomantic/node_modules/when/lib/makePromise.js:845:30)    
    at runContinuation1 (/kerwin612/.linux/ws/gitea/web_src/fomantic/node_modules/when/lib/makePromise.js:804:4) 
Potentially unhandled rejection [4] Error: Error in plugin "gulp-less"
Message:                                                             
    Operation on an invalid type in file /kerwin612/.linux/ws/gitea/web_src/fomantic/node_modules/fomantic-ui/src/themes/default/elements/list.variables line no. 48
Details:                                    
    type: Operation                                                       
    filename: /kerwin612/.linux/ws/gitea/web_src/fomantic/node_modules/fomantic-ui/src/themes/default/elements/list.variables
    index: 1081                                                                      
    line: 48                                                           
    column: 15                                        
    callLine: NaN                                     
    callExtract: undefined                            
,@contentLineHeight: @itemLineHeight;
    lineNumber: 48                                    
    fileName: /kerwin612/.linux/ws/gitea/web_src/fomantic/node_modules/fomantic-ui/src/themes/default/elements/list.variables

    at DestroyableTransform.errorHandler (/kerwin612/.linux/ws/gitea/web_src/fomantic/node_modules/fomantic-ui/tasks/config/tasks.js:124:19)
    at DestroyableTransform.emit (node:events:525:35)
    at DestroyableTransform.emit (node:domain:489:12)
    at onwriteError (/kerwin612/.linux/ws/gitea/web_src/fomantic/node_modules/readable-stream/lib/_stream_writable.js:449:12)
    at onwrite (/kerwin612/.linux/ws/gitea/web_src/fomantic/node_modules/readable-stream/lib/_stream_writable.js:470:11)
    at WritableState.onwrite (/kerwin612/.linux/ws/gitea/web_src/fomantic/node_modules/readable-stream/lib/_stream_writable.js:180:5)
    at DestroyableTransform.afterTransform (/kerwin612/.linux/ws/gitea/web_src/fomantic/node_modules/readable-stream/lib/_stream_transform.js:93:3)
    at /kerwin612/.linux/ws/gitea/web_src/fomantic/node_modules/gulp-less/index.js:58:14
    at tryCatchReject (/kerwin612/.linux/ws/gitea/web_src/fomantic/node_modules/when/lib/makePromise.js:845:30)
    at runContinuation1 (/kerwin612/.linux/ws/gitea/web_src/fomantic/node_modules/when/lib/makePromise.js:804:4)
[09:02:56] The following tasks did not complete: build, build-css, <series>, <series>, Building uncompressed CSS, Building compressed CSS
[09:02:56] Did you forget to signal async completion?
make: *** [Makefile:947: fomantic] Error 1

Because I think this change may be more and more difficult to maintain, so I did not continue, as I replied earlier, I don't quite agree with this modification.

wxiaoguang · 2023-09-13T02:26:26Z

In fact, I have tried, as I said, because of internal dependencies, resulting in changes will need to do all kinds of compatibility;

Thank you for sharing your findings. Although I have read your comment carefully, my initial thought is "we can override the computed variables together". But it looks that it is more complex than it should be.

TBH, there is still another problem in this PR: the .ui.list .item styles don't seem good to mix with ui.input > input, which have other unrelated styles (the comment for input doesn't match list either)

puni9869 · 2023-09-13T05:28:28Z

@kerwin612 changes looks good code viz but due to language barriers could you add some context about this Pr in the description or in PR title. Easy for others too look into.

Thanks for this change. Appreciate.
Let's roll this change.

🚀

Backport.

silverwind · 2023-09-13T09:01:20Z

I would prefer to change @itemLineHeight variable.

@silverwind what do you think?

If it works, sure. My experience with Fomantic var overrides is that some work, some don't for unknown reason. I think this one doesn't.

GiteaBot · 2023-09-13T09:08:54Z

I was unable to create a backport for 1.20. @kerwin612, please send one manually. 🍵

go run ./contrib/backport 26961
...  // fix git conflicts if any
go run ./contrib/backport --continue

* giteaofficial/main: Display all user types and org types on admin management UI (go-gitea#27050) Apply lng2020 to maintainers (go-gitea#27068) Fix incorrect default branch label while switching between branches (go-gitea#27053) set version in snapcraft yaml Replace 'userxx' with 'orgxx' in all test files when the user type is org (go-gitea#27052) [skip ci] Updated translations via Crowdin Load reviewer before sending notification (go-gitea#27063) bump all nightly builds to 16gb Show the repo count in code tab on both user profile and org page. (go-gitea#27048) Fix Fomantic's line-height causing vertical scrollbars to appear (go-gitea#26961) Dashboard context dropdown position fix on landing page in mobile view. (go-gitea#27047)

Fix UI anomalies

a7e49fe

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 7, 2023

pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Sep 7, 2023

silverwind reviewed Sep 7, 2023

View reviewed changes

web_src/css/base.css Outdated Show resolved Hide resolved

techknowlogick added type/bug topic/ui Change the appearance of the Gitea UI labels Sep 7, 2023

kerwin612 added 2 commits September 8, 2023 08:30

Adjusted as @silverwind suggested

cf34473

Merge branch 'main' into patch-2

f99c201

wxiaoguang approved these changes Sep 8, 2023

View reviewed changes

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 Sep 8, 2023

wxiaoguang self-requested a review September 8, 2023 04:08

GiteaBot added lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Sep 8, 2023

kerwin612 added 3 commits September 9, 2023 21:56

Merge branch 'main' into patch-2

0fa29b5

Adjusted as @wxiaoguang suggested

a495f3a

Merge branch 'main' into patch-2

600f1ac

Merge branch 'main' into patch-2

24c1f6b

Merge branch 'main' into patch-2

42647fb

kerwin612 added 2 commits September 13, 2023 10:39

Adjusted as @wxiaoguang suggested

bbb20af

Merge branch 'patch-2' of github.com:kerwin612/gitea into patch-2

2651757

pull-request-size bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Sep 13, 2023

wxiaoguang approved these changes Sep 13, 2023

View reviewed changes

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 Sep 13, 2023

puni9869 approved these changes Sep 13, 2023

View reviewed changes

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 Sep 13, 2023

puni9869 added the backport/v1.20 This PR should be backported to Gitea 1.20 label Sep 13, 2023

kerwin612 changed the title ~~Fix UI anomalies~~ Fix Fomantic's line-height causing vertical scrollbars to appear Sep 13, 2023

lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Sep 13, 2023

lunny added this to the 1.21.0 milestone Sep 13, 2023

lunny enabled auto-merge (squash) September 13, 2023 08:35

Merge branch 'main' into patch-2

3126333

lunny merged commit a38eca3 into go-gitea:main Sep 13, 2023
26 checks passed

GiteaBot added backport/manual No power to the bots! Create your backport yourself! and removed reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. labels Sep 13, 2023

kerwin612 deleted the patch-2 branch September 13, 2023 09:30

go-gitea locked as resolved and limited conversation to collaborators Dec 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Fomantic's line-height causing vertical scrollbars to appear #26961

Fix Fomantic's line-height causing vertical scrollbars to appear #26961

kerwin612 commented Sep 7, 2023 •

edited

Loading

kerwin612 commented Sep 8, 2023

wxiaoguang left a comment •

edited

Loading

wxiaoguang commented Sep 8, 2023 •

edited

Loading

wxiaoguang commented Sep 12, 2023 •

edited

Loading

kerwin612 commented Sep 12, 2023

wxiaoguang commented Sep 13, 2023

kerwin612 commented Sep 13, 2023

wxiaoguang commented Sep 13, 2023 •

edited

Loading

puni9869 commented Sep 13, 2023

silverwind commented Sep 13, 2023 •

edited

Loading

GiteaBot commented Sep 13, 2023

Fix Fomantic's line-height causing vertical scrollbars to appear #26961

Fix Fomantic's line-height causing vertical scrollbars to appear #26961

Conversation

kerwin612 commented Sep 7, 2023 • edited Loading

kerwin612 commented Sep 8, 2023

wxiaoguang left a comment • edited Loading

Choose a reason for hiding this comment

wxiaoguang commented Sep 8, 2023 • edited Loading

wxiaoguang commented Sep 12, 2023 • edited Loading

kerwin612 commented Sep 12, 2023

wxiaoguang commented Sep 13, 2023

kerwin612 commented Sep 13, 2023

wxiaoguang commented Sep 13, 2023 • edited Loading

puni9869 commented Sep 13, 2023

silverwind commented Sep 13, 2023 • edited Loading

GiteaBot commented Sep 13, 2023

kerwin612 commented Sep 7, 2023 •

edited

Loading

wxiaoguang left a comment •

edited

Loading

wxiaoguang commented Sep 8, 2023 •

edited

Loading

wxiaoguang commented Sep 12, 2023 •

edited

Loading

wxiaoguang commented Sep 13, 2023 •

edited

Loading

silverwind commented Sep 13, 2023 •

edited

Loading