Closed cjolif closed 12 years ago
The "inherited" button will show/hide these values, but it also hides everything that's inherited in a normal way (like how Dialog inherits "id" from _WidgetBase).
It's hard for me to do more because the parser output doesn't differentiate between 3rd party modules and pure ancestors.
I am going to comment on csnover/js-doc-parse#52 as well, because if you look at _WidgetBase, it is simply mis-flagging things as "inherited" versus "extended" or "mixed-in" or whatever. I have added functionality to the inlining to differentiate between "inherited" and "foreign", it would in my mind make sense to make "inherited" only mean properly part of the declaration chain.
You mean that you don't want Menu to list that it inherits _TemplatedMixin, because it's a mixin? What's the point?
No I don't want dijit/_WidgetBase to say it inherits colspan
from dojox/layout/TableContainer, which it does now, or closable
from dijit/layout/StackContainer. Because it doesn't inherit those from there. They are extensions that only happen when the other module is loaded.
I want things that are inherited to be properly marked as so and things that are not inherited not to be marked as so.
My point is that only 10% of the properties are inherited. 85% are mixed in, and the remaining 5% are from "third party modules". So be exact in what you are asking for.
I understand...
If you look at dijit/_WidgetBase
though, that is clearly not the right ratio, and it will be wholly confusing to anyone. Of the properties in _WidgetBase, I count 19 declared in the class, 0 inherited, 0 mixed-in and 29 extensions (which are flagged as "inherited: true"). No matter how you define it, right now "inherited" is not accurate. In fact it is meaningless.
To be more specific, anything that is in the declaration's superclass should be flagged separately from something that is derived via extend. Whatever it should be called, I don't care, but right now, they cannot be distinguished.
// Anything here from the superclasses should be "inherited"
var myClass = declare(["base/class1", "base/class2"], {
// ...
});
// Shouldn't be flagged "inherited"
lang.extend(myClass, {
// ...
});
Anyway I can hack something in the exporter to flag those third party fields differently, and then do something appropriate in the viewer. Either a hide button, or perhaps a separate section for each third party module.
I think the question is really whether or not the current module directly or indirectly requires the module that defines the property.
PS: actually it's easier to check in reverse, if the module that defines the property directly requires the module that defines the class, then the property is third-party.
See what you think.
Note this feature also requires https://github.com/wkeese/js-doc-parse/commit/c841836009b2b13db8cd9f0d028985f8b81be3d2,
Exactly what I had in mind! Looking at dijit/_WidgetBase and dijit/_Widget now "make sense" to me. I will need to see how this is manifesting itself in the REST service.
The only remaining question I would have about this, is that extended methods/properties/events are not propagated down to the descendants. Personally I am comfortable with that, but, for example, you will see some of the code base is extending dijit/_WidgetBase
and dijit/_Widget
and none of those extensions are visible in a descendent class.
This is one of the reasons to have the Reference Guide still, because it is far easier to explain in the dijit/layout/StackedContainer page what is being extended in dijit/_WidgetBase, why and how to use those extensions. It would be a lot of information overload if the API Viewer propagated everything to the descendent classes.
How many of the third-party properties added to _WidgetBase are there for the benefit of the parent widget (ex: layoutAlign), and how many are true extensions to _WidgetBase, like the stuff from _BidiSupport?
Because the properties for the parent widget (arguably) shouldn't be listed on the _WidgetBase documentation page at all, but rather just in the documentation for the parent widget (ex: BorderContainer). The properties are being added to _WidgetBase mainly to make the parser work, but we could easily add a [hide] tag for those properties and make the API viewer not show them.
Note also that in 2.0 we'll probably stop mixing properties into _WidgetBase altogether, because it's only necessary for the deprecated
<div dojoType=dijit.layout.ContentPane layoutAlign=top>
syntax and not the standardista friendly
<div data-dojo-type="dijit/layout/ContentPane" data-dojo-props="layoutAlign: 'top'">
cc @AdrianVasiliu
@wkeese I do agree the parser problem will disappear in 2.0 but still even if that properties are for the parent widget, the user will actually have to define them on the child widgets and so I think it is important to get a sense of that when reading the regular API widget documentation to know that you will be able to specify information for your parent widget on the child widget? That said I agree having too many of these properties on _WidgetBase is not easy to read.
This is obviously not for now but I'm wondering if we should not have something a bit similar to Java constraints which are a generic objects passed on the parent layout from the children.
Maybe something like
<div data-dojo-type="MyContainer">
<div data-dojo-type="MyChild" data-dojo-props="regular child properties"
data-dojo-constraints="object configuring this child for its parent"></div>
<div>
or if we don't want to introduce a new data-dojo-something, have it as a regular data-dojo-props:
<div data-dojo-type="MyContainer">
<div data-dojo-type="MyChild"
data-dojo-props="regular child properties,
constraints: object configuring this child for its parent"></div>
<div>
On this side topic, I am not sure what that would improve. I can understand why Java needs it, but with JavaScript's loosely defined objects, we should embrace the language versus artificially constraining it. If there was a distinct advantage that isn't obvious to me, then I would be for it.
On Mon, Jul 9, 2012 at 6:56 PM, Christophe Jolif < reply@reply.github.com
wrote:
even if that properties are for the parent widget, the user will actually have to define them on the child widgets and so I think it is important to get a sense of that when reading the regular API widget documentation
So, something like this in BorderContainer's API doc isn't sufficient?
// description: // ... // The child widgets must have the following attributes: // // - layoutAlign: this is for ...
@wkeese this is obviously better than nothing and that's maybe the best solution for now.
That said when I look at the API doc of an object I like to see on that object (an not its potential parent) that I can set properties for its parent directly on the widget itself. Obviously for someone knowing Dojo already that is not a big gain but for someone new this is not obvious that you must look at parent's doc to learn about properties of the children.
To answer @kitsonk I don't think this contradicts JavaScript loosely defined objects to have a holder property for this as this property would be loosely defined. It would just be a "marker" that would let the user know "hey this is this property that holds what needs to go to the parent widget if you want to know more about what can go in there, checkout your parent doc". (for example).
when I look at the API doc of an object I like to see on that object (an not its potential parent) that I can set properties for its parent directly on the widget itself
I can see the value in that. The problem is that it doesn't scale well, as shown by all the properties added to _WidgetBase. So the question is, does displaying those parent-child properties on the child do my harm than good?
PS: from Kitson's comment below he clearly believes "more harm than good".
@cjolif that is what worries me a bit, if we auto include those extensions, then EVERY Dijit will have a property list that is as long as your arm. Something like 30/40 extended properties and would defeat the purpose/confuse the API guide. I think it is part of why we need the Reference Guide. How is someone who is new to Dojo, who sees the property "colspan" on a Dijit understand that that is only applicable when using a semi-abandoned DojoX layout module? It is "bad enough" that it is showing in _WidgetBase, but to propagate it will make the API documents almost useless in my opinion.
Of course, we might be too "close" to it and almost need someone who is relatively new to Dojo to give an opinion. Tomorrow night I am speaking at the London AJAX User Group, I might find a guinea pig there.
The other stuff for 2.0 is another interesting debate for a different day! :-)
By default if a 3rd party module is patching your class to add more properties/methods on it it will show up in the documentation.
This is good in general. However this might lead in certain cases to tons of additional properties not facilitating reading it. The viewer could provide a switch to hide those.
See also csnover/js-doc-parse#52