Fix thick scrollbar on filter menu #10835

CirnoT · 2020-03-26T11:45:10Z

Before:

After:

CirnoT · 2020-03-26T18:02:28Z

Unsure what happened with CI here, looks like some UTF-8 issue?

zeripath · 2020-03-26T19:19:30Z

@CirnoT What browser did you see this on?

zeripath

We should really be ensuring that a scrollbar doesn't need to be shown I think...

@silverwind What do you think?

zeripath · 2020-03-26T19:21:36Z

@go-gitea/mergers feel free to dismiss my request changes if I have missed something here.

CirnoT · 2020-03-26T20:35:05Z

@CirnoT What browser did you see this on?

Firefox, Windows 10

We should really be ensuring that a scrollbar doesn't need to be shown I think...

That might be little un-intuitive as there would be no indication to user that the area is scrollable. The scrollbar shows only when there's additional content hidden as it should, of course.

silverwind · 2020-03-26T20:56:54Z

If content overflows unexpectedly, it's better to fix the underlying root issue why it overflows in first place than to use overflow: hidden which I see as a last-ditch effort. Based on the screenshot, I think some style might limit the horizontal width of the popup. Try removing that limit and let it naturally expand based on content.
scrollbar-width is Firefox-only currently. There are ways to do it on webkit browsers too but it's very finicky and I would not recommend it there. That said, I think I'd be slightly in favor of adding site-wide thin scrollbars, e.g.

*:not(select) {
  scrollbar-width: thin;
}

(taken from https://github.com/StylishThemes/Overlay-Scrollbars/blob/master/github-overlay-scrollbars.user.css)

silverwind

See above

CirnoT · 2020-03-26T22:06:15Z

If content overflows unexpectedly, it's better to fix the underlying root issue why it overflows in first place than to use overflow: hidden which I see as a last-ditch effort. Based on the screenshot, I think some style might limit the horizontal width of the popup. Try removing that limit and let it naturally expand based on content.

I am afraid that would be hard to achieve, the width is decided by browser dynamically and I see no direct CSS stylesheet that would limit it. It is properly calculated based on the content, however the vertical scrollbar eats away at the pixels causing the element body inside to overflow.

The one solution I could see helping it would be to remove width: 100px from .ui.menu .ui.dropdown.item .menu .item as well as override Fomantic's own margin: 0; on .ui.menu .ui.dropdown .menu > .item

If you believe this is worth blocking this PR for, I fully understand that and step down - this was supposed to be a quick fix for visual glitch with rather deep roots, I have no intentions on digging any deeper into trying to hack unfamiliar to me framework.

CirnoT · 2020-03-26T22:13:09Z

For what it's worth, from quick check it does seem to be Firefox specific (though how much that is because of specifics in rendering engine and how much due to different scrollbar styling applied is unknown) and partially caused by white-space: nowrap; applied by Fomantic to these menu elements as well

guillep2k · 2020-03-27T03:16:42Z

Don't sweat about the CI failing. It happens from time to time. I've just restarted the task.

codecov-io · 2020-03-27T03:43:25Z

Codecov Report

Merging #10835 into master will increase coverage by 0.24%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #10835      +/-   ##
==========================================
+ Coverage   43.56%   43.81%   +0.24%     
==========================================
  Files         589      590       +1     
  Lines       82700    83387     +687     
==========================================
+ Hits        36031    36535     +504     
- Misses      42192    42331     +139     
- Partials     4477     4521      +44

Impacted Files	Coverage Δ
modules/auth/repo_form.go	`41.29% <0.00%> (-3.16%)`	⬇️
services/pull/check.go	`52.43% <0.00%> (-3.05%)`	⬇️
modules/git/command.go	`86.95% <0.00%> (-2.61%)`	⬇️
modules/indexer/issues/indexer.go	`56.19% <0.00%> (-1.33%)`	⬇️
services/pull/temp_repo.go	`31.62% <0.00%> (ø)`
models/migrations/v132.go	`0.00% <0.00%> (ø)`
modules/convert/convert.go	`75.65% <0.00%> (+0.81%)`	⬆️
services/pull/pull.go	`35.88% <0.00%> (+0.88%)`	⬆️
models/error.go	`30.16% <0.00%> (+0.93%)`	⬆️
routers/repo/setting_protected_branch.go	`42.45% <0.00%> (+1.02%)`	⬆️
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 52cfd27...3c0417a. Read the comment docs.

silverwind · 2020-03-28T08:09:36Z

I can't seem to quite replicate the horizontal scrollbar. For some reason, this Use alt.. hint does not show for me. What I do see is that items are limited to 250px via this:

.repository .select-label .item {
    max-width: 250px;
    overflow: hidden;
    text-overflow: ellipsis;
}

This seems to be the rule that eliminates the horizontal on these. I think one solution could be to just ensure the .item class is present on all children.

Alternatively, can you post the HTML you see when the horizontal scrollbar is visible?

CirnoT · 2020-03-28T19:39:44Z

The rule you linked does not apply to these elements as they are never inside container with class select-label.

If you looked at screenshots you would see that the issue happens on "Assignee" where hint is not shown at all, as such it is red herring.

Have HTML as requested, tho i have no idea why it would be necessary as it can be easily replicated.

<div class="menu transition visible" tabindex="-1" style="display: block !important;">
  <a class="item" href="*****" tabindex="-1" role="menuitem" id="menuitem_37">All assignees</a>
  <a class=" item" href="*****" tabindex="-1" role="menuitem" id="menuitem_38"><img src="/user/avatar/*****/-1"> *****</a>
  <a class=" item" href="*****" tabindex="-1" role="menuitem" id="menuitem_39"><img src="/user/avatar/*****/-1"> *****</a>
  <a class=" item" href="*****" tabindex="-1" role="menuitem" id="menuitem_40"><img src="/user/avatar/*****/-1"> *****</a>
  <a class=" item" href="*****" tabindex="-1" role="menuitem" id="menuitem_41"><img src="/user/avatar/*****/-1"> *****</a>
  <a class=" item" href="*****" tabindex="-1" role="menuitem" id="menuitem_42"><img src="/user/avatar/*****/-1"> *****</a>
  <a class=" item" href="*****" tabindex="-1" role="menuitem" id="menuitem_43"><img src="/user/avatar/*****/-1"> *****</a>
  <a class=" item" href="*****" tabindex="-1" role="menuitem" id="menuitem_44"><img src="/user/avatar/*****/-1"> *****</a>
</div>

CirnoT · 2020-03-28T19:43:29Z

Looking at it, it would seem that ensuring select-label on container div does solve the issue everywhere except on Label search where hint is shown.

Because the hint uses <span>, adding item on it and ensuring select-label on parent does solve the issue, however at the same time it results in hint being partially hidden.

<span class="info">Use <code>alt</code> + <code>click/enter</code> to exclude labels</span>

silverwind · 2020-03-29T14:46:36Z

I think we can use this general approach to limit width of all direct children to .menu on the sidebar:

.metas .menu > * {
  max-width: 250px;
  overflow: hidden;
  text-overflow: ellipsis;
}

Remove the .repository .select-label .item after adding that.

Edit. Nevermind I see we're talking about different parts. I thought this was the issue sidebar.

silverwind · 2020-03-29T14:56:59Z

This seems to work for me:

.metas .menu.transition > *, #issue-filters .menu.transition > * {
  max-width: 250px;
  overflow: hidden;
  text-overflow: ellipsis;
}

Fomantic-UI does not seem to have a distinct class for dropdowns, so hopefully the .transition class addition will help with only matching those.

CirnoT · 2020-03-29T17:22:05Z

This does seem to work everywhere aside for the hint, which is not affected at all despite CSS being applied properly.

<span class="info">Use <code>alt</code> + <code>click/enter</code> to exclude labels</span>

Is there any reason why we can't ensure select-label to be present on container for these dropdowns?

silverwind · 2020-03-29T19:29:16Z

I would aim for a more general solution for all dropdowns. According to this a dropdown is defined by .dropdown > .menu, so I think we can just use:

.dropdown > .menu > * {
  max-width: 300px;
  overflow-x: hidden;
  text-overflow: ellipsis;
}

I'm pretty sure there are no dropdowns around that would require more width. Note I bumped the width by 50px as I think we can show a bit more of overly long labels and to possibly cover other dropdowns that might need more space. The .info needs a width adjustment to stretch the parent:

.repository .filter.menu.labels .label-filter .menu .info {
  width: 100%;
}

CirnoT · 2020-03-30T12:20:38Z

It does work, however the tip ends up like this:

Unsure if we can do anything about it however, maybe it needs re-wording to fit in available space. Particularly the click/enter part seems redundant. Possibly

Hold alt while selecting to exclude

silverwind · 2020-03-30T12:43:49Z

Hold alt while selecting to exclude

Sounds good to me

CirnoT · 2020-03-30T17:43:32Z

Should I close this PR and you will open new one with changes? @silverwind

silverwind · 2020-03-30T18:09:59Z

Yes, I'll open a new one soon.

CirnoT · 2020-03-31T12:00:02Z

See #10897

Fixes: go-gitea#10835

* Fix scrollbar issues in dropdowns Fixes: #10835 * remove wrapping * grammar Co-authored-by: guillep2k <[email protected]>

Fix thick scrollbar on filter menu

1aba2f4

lafriks added the topic/ui Change the appearance of the Gitea UI label Mar 26, 2020

lafriks added this to the 1.12.0 milestone Mar 26, 2020

lafriks approved these changes Mar 26, 2020

View reviewed changes

GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Mar 26, 2020

zeripath approved these changes Mar 26, 2020

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 Mar 26, 2020

zeripath requested changes Mar 26, 2020

View reviewed changes

Merge branch 'master' into patch-1

3c0417a

silverwind requested changes Mar 26, 2020

View reviewed changes

silverwind mentioned this pull request Mar 30, 2020

Fix scrollbar issues in dropdowns #10897

Merged

CirnoT closed this Mar 31, 2020

silverwind added a commit to silverwind/gitea that referenced this pull request Apr 2, 2020

Fix scrollbar issues in dropdowns

31a12dd

Fixes: go-gitea#10835

guillep2k added a commit that referenced this pull request Apr 4, 2020

Fix scrollbar issues in dropdowns (#10897)

6cf71ee

* Fix scrollbar issues in dropdowns Fixes: #10835 * remove wrapping * grammar Co-authored-by: guillep2k <[email protected]>

CirnoT deleted the patch-1 branch April 21, 2020 15:29

go-gitea locked and limited conversation to collaborators Nov 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix thick scrollbar on filter menu #10835

Fix thick scrollbar on filter menu #10835

CirnoT commented Mar 26, 2020

CirnoT commented Mar 26, 2020

zeripath commented Mar 26, 2020

zeripath left a comment

zeripath commented Mar 26, 2020

CirnoT commented Mar 26, 2020 •

edited

Loading

silverwind commented Mar 26, 2020

silverwind left a comment

CirnoT commented Mar 26, 2020

CirnoT commented Mar 26, 2020 •

edited

Loading

guillep2k commented Mar 27, 2020

codecov-io commented Mar 27, 2020

silverwind commented Mar 28, 2020 •

edited

Loading

CirnoT commented Mar 28, 2020

CirnoT commented Mar 28, 2020

silverwind commented Mar 29, 2020 •

edited

Loading

silverwind commented Mar 29, 2020

CirnoT commented Mar 29, 2020 •

edited

Loading

silverwind commented Mar 29, 2020 •

edited

Loading

CirnoT commented Mar 30, 2020 •

edited

Loading

silverwind commented Mar 30, 2020

CirnoT commented Mar 30, 2020

silverwind commented Mar 30, 2020

CirnoT commented Mar 31, 2020

Fix thick scrollbar on filter menu #10835

Fix thick scrollbar on filter menu #10835

Conversation

CirnoT commented Mar 26, 2020

CirnoT commented Mar 26, 2020

zeripath commented Mar 26, 2020

zeripath left a comment

Choose a reason for hiding this comment

zeripath commented Mar 26, 2020

CirnoT commented Mar 26, 2020 • edited Loading

silverwind commented Mar 26, 2020

silverwind left a comment

Choose a reason for hiding this comment

CirnoT commented Mar 26, 2020

CirnoT commented Mar 26, 2020 • edited Loading

guillep2k commented Mar 27, 2020

codecov-io commented Mar 27, 2020

Codecov Report

silverwind commented Mar 28, 2020 • edited Loading

CirnoT commented Mar 28, 2020

CirnoT commented Mar 28, 2020

silverwind commented Mar 29, 2020 • edited Loading

silverwind commented Mar 29, 2020

CirnoT commented Mar 29, 2020 • edited Loading

silverwind commented Mar 29, 2020 • edited Loading

CirnoT commented Mar 30, 2020 • edited Loading

silverwind commented Mar 30, 2020

CirnoT commented Mar 30, 2020

silverwind commented Mar 30, 2020

CirnoT commented Mar 31, 2020

CirnoT commented Mar 26, 2020 •

edited

Loading

CirnoT commented Mar 26, 2020 •

edited

Loading

silverwind commented Mar 28, 2020 •

edited

Loading

silverwind commented Mar 29, 2020 •

edited

Loading

CirnoT commented Mar 29, 2020 •

edited

Loading

silverwind commented Mar 29, 2020 •

edited

Loading

CirnoT commented Mar 30, 2020 •

edited

Loading