Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
PG Lewis
@pglewis
but definitely let me know what you're brainstorming, I can modify my course of action here to accommodate if needed
since you said "I can do that quick", I'd have to assume it's not a giant refactor. Either that or just a case of optimism ;)
Justin Sternberg
@jtsternberg
Nevermind... the modifications to the tests are good
PG Lewis
@pglewis
I needed to give explicit field names to the data arrays, that's the one oddball thing
but it makes sense that it should be that way... creation of the field is going to name them by field ID and not the original indexes 0, 1, 2 when omitted
God knows it'll find the one guy out there relying on those index values upon release though
Justin Sternberg
@jtsternberg
No, that's the way it should be. I'm actually working on a method that will convert fields arrays to using the add_field methods
PG Lewis
@pglewis
it'll still take field arrays without names, you just can't rely on those numeric indexes the way I've refactored... fields always get the name set on creation, and they're always created
but I wanna make sure I stay on the same page as you
if you're reworking add_field... well, obviously I'm hacking add_field too
in fact, all the nesting I built in the example was done exclusively via add_field
Justin Sternberg
@jtsternberg
no, this is what I'm working on.. sounds like maybe a little overlap. Let me know: http://b.ustin.co/1bcue
PG Lewis
@pglewis
since fields can be groups now, I can call add_field on a field group, not just a metabox
I didn't use add_group_field once for the example in the screenshot
Justin Sternberg
@jtsternberg
hmmm.. let me try to re-do that diff
PG Lewis
@pglewis
this gives you an idea of how far removed my branch is:
$meta_box doesn't even exist
and everything to do with fields and field collections is moved out to the new class
when boiled down at that point, it's basically a bunch of getters and setters and nonce management
PG Lewis
@pglewis
and show_form, of course
Justin Sternberg
@jtsternberg
does that conflict with your stuff?
basically, if they add fields an an old-school array, it loops through them and adds them the right way
PG Lewis
@pglewis
probably, but it's not exactly a giant changeset
I'm trying to merge back up from your trunk, to ours, and up to my branch
test changes I put in our trunk and submit the PR from there
so if you make changes I'll end up merging them in on my own, if that means refactoring it for working the "new way"
Justin Sternberg
@jtsternberg
that's a lot of setters/getters. Is there any reason there should be a method for all of them, vs one setter/getter?
PG Lewis
@pglewis
my add_fields isn't even in cmb2
because it's easier to manage in the long run, IME
most of the tedium is in creating them, and phpStorm will generate skeletons for every class property I highlight
Justin Sternberg
@jtsternberg
how/why? (not being argumentative, just want to understand)
PG Lewis
@pglewis
if you have all your logic for all your classes properties in one method, that method can grow pretty big
and 9 times out of ten it's just if/elseif or switch/case anyway
also benefits from doing it that way are autocompletion and phpDoc with a keystroke
having those allows you to change the whole underlying implementation later if you want to
the getters and setters are what are exposed and used on the consuming side, and hide the details so you're more free to change them in the future
though a single getter/setter does that too
Justin Sternberg
@jtsternberg
yah, that's my main question. Why not one (or fewer)
PG Lewis
@pglewis
let me turn it on it's head for a moment, what's the benefit of not having separate ones?
PG Lewis
@pglewis
with individual getters and setters you can phpDoc the return values properly for each one
Justin Sternberg
@jtsternberg
yah, that's true. Curious, how does this handle custom metabox properties?
PG Lewis
@pglewis
this in turn means my IDE knows when I call get_field_objects() and foreach through it, each element is a Field object
and i get autocomplete on the field object as a freebie
Justin Sternberg
@jtsternberg
because users can (and do) add arbitrary data to the cmb array and then pull it out elsewhere
Justin Sternberg
@jtsternberg
since we're renaming things, I think we definitely should rename the cmb_id property/methods to just id