This builds out and implements PHUIPropertyListView (container) and PHUIPropertyListItemView (section) as well as adding tabs.
Details
Details
- Reviewers
epriestley btrahan - Commits
- Restricted Diffusion Commit
rP97c690fc0f52: PHUIPropertyListView
Tested each page I edited with the exception of Releeph and Phortune, though those changes look ok to me diff wise. Updated examples page with tabs.
Diff Detail
Diff Detail
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
Comment Actions
All I'm doing at the moment is making tabs available. What I'll need you to do is some basic show/hide of div content with each tab, which I'm not sure how to do is a logical/easy manner.
Comment Actions
Cool, let me take a crack at the JS. I might end up making changes like this, do they sound reasonable?
- Rename PHUIPropertyListItemView back to PHUIPropertyListView, and rename PHUIPropertyListView to something like PHUIPropertyListGroupView? At first glance, "ListItemView" is kind of ambiguous and confusing to me, given that it does not mean "list item" (i.e., an item in a list).
- Similarly, make the method on the grouped view addPropertyList() instead of addItem(). This will probably be where you set the tab, too, e.g. addPropertyList($list, $optional_tab = null).
- Keep the code that combines PropertyList with ActionList inside PHUIObjectBoxView, instead of where the property lists are built. In particular, we want a single list of actions to persist across all the tabs, right? I want to avoid unnecessarily rendering 5 copies of it if we have 5 tabs, because this will make other things we might want to build later (say, ajax for "Unsubscribe" which updates the action) more complicated. I'll probably pull the actual rendering out a little bit.
If any of that seems crazy, yell at me.
| src/view/layout/PhabricatorPropertyListView.php | ||
|---|---|---|
| 120–122 | TODO, or dead code? | |
| src/view/phui/PHUIObjectBoxView.php | ||
| 13–16 | "convenience" | |
Comment Actions
I agree with most, I think. I am bad at naming stuff.
On the persistent actions, it's not something I thought would be useful. The main reasons being I wanted the extra space for mobile displays. Things under tabs like Lint/Unit/CI could all benefit from the extra space and it also affords the flexibility if needed to have different actions per tab. This is how I ended up moving everything into PropertyListView.
I suppose then tabs should switch individual PropertyListViews and not be 'in' them.
I also didn't think I should special case properties and actions to objectbox, in case for future flexibility.
Comment Actions
I can G-Hang if that helps sync quicker, I might not fully understand all your suggestions.
Comment Actions
- PHUIPropertyGroupView
- PHUIPropertyListView
- PHUIPropertyGroupView->addPropertyList
- addPropertyListItem -> addPropertyList
Comment Actions
I hit everything except the last suggestion. I think we settled on actionlist only on the first tab (though you could have different ones on different tabs if you really really wanted to). There are a few cases we have multiple action lists on a page, so we're not setting them on the object box, but on the propertylist itself. Mainly the /edit/repository/x/ should look much better now.
Some minor cleanup still needed on CSS in a few places like custom profiles, but nothing urgent. Did you want to re-check this or just land it. I spun up all the main pages (tasks, diffs, phriction, profiles).