Fixes overflowing of AphrontSideNavFilterView on small screens and tidies up nav bar styles
Needs ReviewPublic
Actions

Authored by bekay on Sat, May 4, 13:21.

Details

Reviewers

None

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15809: Homepage will overflow on small screens when feed content is too long

Summary

Switch the two column layout of AphrontSideNavFilterView (used in pretty much most views) from table to flex display. The former CSS rendering could lead to overflowing of the entire page on small screens if very wide unbreaking content is displayed (example described here: T15809).

While I was at it I have found some unused code. The CSS file phabricator-nav-view.css consisted almost only of .phabricator-side-menu-home class selectors. This is an ancient class (there at least since 2013) not used anymore:

https://we.phorge.it/source/phorge/browse/master/src/applications/directory/controller/PhabricatorDirectoryController.php;919bd4a03499305093d8882952a9dd3dac1c55a9$169

So I have removed this CSS component and saved the few still used declarations to the main phui-basic-nav-view.css.

There was some unused code in AphrontSideNavFilterView.php too.

Fixes T15809

Test Plan

Look at this video to understand the problem:

F2178277

Go in a Task comment and mention a task with a very-long title. Have phd enabled to generate feeds. Visit homepage with feeds:

See that you cannot reproduce anymore.

Browse pages with the right nav bar with many devices and look if the layout looks fine and not broken.

Tested the homepage, scaling from mobile to desktop, using:

Firefox 124 64bit on GNU/Linux (snap)
Chromium 124 64bit on GNU/Linux (snap)
...

Tested other elements, scaling from mobile to desktop, still working correctly:

popup dialogs
right floating menu
top curtain
- notifications
- Conpherence chats
- unresolved setup issues
- search omnibox

Diff Detail

Repository

rP Phorge

Branch

master

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 1222
Build 1222: arc lint + arc unit

Event Timeline

bekay created this revision.Sat, May 4, 13:21

Owners added a reviewer: O1: Blessed Committers.Sat, May 4, 13:21

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan, tobiaswiese. · View Herald TranscriptSat, May 4, 13:21

bekay requested review of this revision.Sat, May 4, 13:21

Harbormaster completed remote builds in B1222: Diff 1934.Sat, May 4, 13:21

bekay edited the summary of this revision. (Show Details)Sat, May 4, 13:25

bekay added a task: T15809: Homepage will overflow on small screens when feed content is too long.

bekay edited the summary of this revision. (Show Details)Sat, May 4, 13:28

screenies?

All described here with screenshot (and now video): T15809

But with my test you can just go to https://we.phorge.it with your mobile and see it for yourself.

Ah, yes thank you for the video. I should have clarified but was asking for screenies of the result after this change. The changes look reasonable but I’m not a regular css user

In D25619#17360, @speck wrote:

Ah, yes thank you for the video. I should have clarified but was asking for screenies of the result after this change. The changes look reasonable but I’m not a regular css user

The site looks the same of course just no overflowing 😋 long unbreaking elements are just cut off. Hopefully no big chance. But I think it should be checked out on some test enviromnents of busy reviewers.

valerio.bozzolan edited the summary of this revision. (Show Details)Thu, May 9, 08:01

valerio.bozzolan edited the test plan for this revision. (Show Details)

Maybe "feature" but note the left sidebar, is not anymore connected to the end of the viewport: