Page MenuHomePhabricator

PHUIPropertyListView
ClosedPublic

Authored by chad on Oct 10 2013, 8:43 PM.
Tags
None
Referenced Files
F14094305: D7283.diff
Mon, Nov 25, 3:15 PM
Unknown Object (File)
Mon, Nov 18, 9:58 PM
Unknown Object (File)
Sun, Nov 10, 4:52 AM
Unknown Object (File)
Fri, Nov 8, 6:57 AM
Unknown Object (File)
Wed, Nov 6, 4:16 AM
Unknown Object (File)
Oct 27 2024, 4:32 AM
Unknown Object (File)
Oct 23 2024, 12:19 PM
Unknown Object (File)
Oct 19 2024, 9:32 PM

Details

Reviewers
epriestley
btrahan
Commits
Restricted Diffusion Commit
rP97c690fc0f52: PHUIPropertyListView
Summary

This builds out and implements PHUIPropertyListView (container) and PHUIPropertyListItemView (section) as well as adding tabs.

Test Plan

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

Branch
prop-list
Lint
Lint Passed
Unit
Tests Passed

Event Timeline

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.

chad updated this revision to Unknown Object (????).Oct 10 2013, 8:49 PM
  • unused if statement

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/phui/PHUIObjectBoxView.php
13

"convenience"

src/view/phui/PHUIPropertyListItemView.php
120–122

TODO, or dead code?

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.

I can G-Hang if that helps sync quicker, I might not fully understand all your suggestions.

Maybe setTabs should just be in ObjectBoxView, that likely makes more sense.

chad updated this revision to Unknown Object (????).Oct 11 2013, 3:51 AM
  • PHUIPropertyGroupView
  • PHUIPropertyListView
  • PHUIPropertyGroupView->addPropertyList
  • addPropertyListItem -> addPropertyList

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).