Fixes T4914. We currently have a finite limit on column displays which caused T4914. This fixes T4914 by no longer using a fluid layout. Rather, we use a fixed column width layout which does not have a 7 column limit. Future work - see T4054 for an example - will likely make the fluid layout thing work with infinite columns, and / or other work may re-jigger project workboards directly.
Details
- Reviewers
epriestley chad - Maniphest Tasks
- T4054: Allow AphrontMultiColumnView to support infiite* and custom* column widths
T4914: Don't let users make more than the supported number of workboards - Commits
- Restricted Diffusion Commit
rP7f13e8a5c576: Workboards - remove 7 column restriction
had a project like in T4914 that wouldn't load and it loaded post this change! added more columns and using javascript inspector noted proper width being set
Diff Detail
- Repository
- rP Phabricator
- Branch
- T4914
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 287 Build 287: [Placeholder Plan] Wait for 30 Seconds
Event Timeline
webroot/rsrc/css/aphront/multi-column.css | ||
---|---|---|
64 | The big concession as I understand it - if you have some glorious 3200pixel display with a hojillion columns, you will see 1600 pixel wide that then snaps to 3200+ based on how many a hojillion really is. I am using a 15" MacbookPro with Retina display though and this is already hugely big. | |
webroot/rsrc/js/core/behavior-multi-column.js | ||
18 ↗ | (On Diff #21213) | My idea was that this works for all amounts of columns, kicking the old can infinitely far down the road as far as functionality is concerned. Not sure if the 200 pixel per column math really holds true, and I suspect you'd like to have widths autoscale based on the viewport and whatnot. |
Can we just make these columns always be 320px-ish wide? The 1 and 2 column cases are fairly silly looking at 100% and 50% width, and things get better at 3-5 columns and then the columns get unusably tiny and finally explode at 7. Then we don't need any JS or magic.
As a point of comparison, Trello columns are always 260px-ish wide.
Any objection to switching to fixed? Did I just write this weird originally by picking fluid?
Yeah I just like fluid, you get more value out of the real estate. Is there a way to do both? Min-width something? I'd have to look at the CSS again
Here's the hacky fixed-width version -- major advantage is no JS stuff.
diff --git a/src/applications/project/controller/PhabricatorProjectBoardViewController.php b/src/applications/project/controller/PhabricatorProjectBoardViewController.php index a697619..2cf8b8e 100644 --- a/src/applications/project/controller/PhabricatorProjectBoardViewController.php +++ b/src/applications/project/controller/PhabricatorProjectBoardViewController.php @@ -89,7 +89,6 @@ final class PhabricatorProjectBoardViewController $board = id(new PHUIWorkboardView()) ->setUser($viewer) - ->setFluidishLayout(true) ->setID($board_id); $this->initBehavior( diff --git a/src/view/layout/AphrontMultiColumnView.php b/src/view/layout/AphrontMultiColumnView.php index e314cb2..a12a4f1 100644 --- a/src/view/layout/AphrontMultiColumnView.php +++ b/src/view/layout/AphrontMultiColumnView.php @@ -46,7 +46,9 @@ final class AphrontMultiColumnView extends AphrontView { $classes[] = 'grouped'; if (count($this->columns) > 7) { - throw new Exception("No more than 7 columns per view."); + if ($this->fluidLayout || $this->fluidishLayout) { + throw new Exception("No more than 7 columns per view."); + } } $classes[] = 'aphront-multi-column-'.count($this->columns).'-up'; diff --git a/src/view/phui/PHUIWorkboardView.php b/src/view/phui/PHUIWorkboardView.php index 06ee373..835a94e 100644 --- a/src/view/phui/PHUIWorkboardView.php +++ b/src/view/phui/PHUIWorkboardView.php @@ -3,8 +3,6 @@ final class PHUIWorkboardView extends AphrontTagView { private $panels = array(); - private $fluidLayout = false; - private $fluidishLayout = false; private $actions = array(); public function addPanel(PHUIWorkpanelView $panel) { @@ -12,16 +10,6 @@ final class PHUIWorkboardView extends AphrontTagView { return $this; } - public function setFluidLayout($layout) { - $this->fluidLayout = $layout; - return $this; - } - - public function setFluidishLayout($layout) { - $this->fluidishLayout = $layout; - return $this; - } - public function addAction(PHUIIconView $action) { $this->actions[] = $action; return $this; @@ -57,12 +45,6 @@ final class PHUIWorkboardView extends AphrontTagView { $view = new AphrontMultiColumnView(); $view->setGutter(AphrontMultiColumnView::GUTTER_MEDIUM); - if ($this->fluidLayout) { - $view->setFluidLayout($this->fluidLayout); - } - if ($this->fluidishLayout) { - $view->setFluidishLayout($this->fluidishLayout); - } foreach ($this->panels as $panel) { $view->addColumn($panel); } diff --git a/webroot/rsrc/css/aphront/multi-column.css b/webroot/rsrc/css/aphront/multi-column.css index 6fe0e6f..2005487 100644 --- a/webroot/rsrc/css/aphront/multi-column.css +++ b/webroot/rsrc/css/aphront/multi-column.css @@ -28,7 +28,7 @@ } .aphront-multi-column-fixed .aphront-multi-column-column-outer { - width: 300px; + width: 280px; } /* flexible, but with a minimum */ diff --git a/webroot/rsrc/css/phui/phui-workpanel-view.css b/webroot/rsrc/css/phui/phui-workpanel-view.css index f1ef1bc..24b4ab2 100644 --- a/webroot/rsrc/css/phui/phui-workpanel-view.css +++ b/webroot/rsrc/css/phui/phui-workpanel-view.css @@ -53,7 +53,7 @@ } .aphront-multi-column-fixed .phui-workpanel-body { - width: 300px; + width: 280px; } .phui-workpanel-body .phui-object-item-list-view {
Specific issues I see with this JS approach are:
- JS is sorta yuck
- I think these columns get unreasonably narrow, particularly with the current card approach:
- At least in Safari, we often end up with a big unused gutter of unused space on the right for me.
- The full-width version with 1 or 2 columns is kind of silly when it's fluid, maybe? Particularly, there's no UI affordance that you can add more columns, since the entire UI is full. In Trello, it's obvious that you can add more columns (Trello also has a specific "Add a column.." affordance).
The silliness is mostly only in the nux though. The point of boards being to make boards. A phantom "add column" could improve new board experience and go away after a panel or two is made.
Or we're overthinking this
Yeah, agreed on overthinking. Whatever its faults, this is clearly better than exploding on 8+ columns, and we have like 10 things in queue to improve boards which will give us plenty of opportunities to fix this properly and do tweaks like empty-board-states.
I'm just wondering when this diff will be landed? We are going to upgrade our Phabricator install once this happens.
fixed width version. we basically just
- stop setting this thing to fluid at the controller level
- NOTE otherwise fluid functionality still as it was before pre patch
- only through 7 column exception on fluid layouts (so not for project workboards)
- tweak fixed CSS width ever so slightly from 300 => 280
...is that right?
As to why I made this change, that was my best understanding of what you all wanted me to do based on our discussion yesterday / the diff comments. I would guess the deeper why there is since columns wont get auto-squished in this fixed layout we probably want to be as wide as minimally possible. That said, I admit I am just trying to follow orders. :D
We should fix the height as well to the browser height so you can see the scroll bar. In this board if you shrink the window, it just terminates the object items and you can't see the scroll bar.
I recall at this point I removed the 'well' around the board columns for simplicity, but it was never used in Workboards, so it never seemed weird. We'll have to bring a well back and absolutely position the height so the scrollbar is always visible.
@chad - is that something you want me to do or are you doing? If the former, see below as I am confused and generally please do call me out directly if there's something you want me to do. :D
(Fix the height of what to the browser height? By "browser height" do you mean the viewport (the size of the viewable area to the user), the height of the entire rendered document, or maybe something else? Also, whatever that answer is, how do you get that in CSS? (I know in JS... :D)
Do you want to bring the well back stylistically, or perhaps its the thing from sentence 1 that we need to fix the height of to the "browser height", and is just a technique to achieve that end?)
I’m working on a fix. This is my fault, I changed this view but never tested it anywhere other than UIExamples. It needs some work to be ready for abused boards.