Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

parser: support data-dojo-mixin? #28

Closed
wkeese opened this issue Oct 23, 2013 · 9 comments
Closed

parser: support data-dojo-mixin? #28

wkeese opened this issue Oct 23, 2013 · 9 comments

Comments

@wkeese
Copy link
Member

wkeese commented Oct 23, 2013

Should we migrate data-dojo-mixins attribute to the new custom element parser? Something like

<d-treemap data-mixins="dtreemap/Keyboard"></d-treemap>

If we want to continue to build components on this idea of pluggable mixins we probably need to have something similar like that in dui to make it easy to that in markup? If not we probably need to revise the idea because this would make it more complicated to use it.

I agree it's a nice feature in Dojo V1 but it doesn't fit into the web components philosophy. Eventually browsers will support document.register() natively, and if we use it then there won't be any way to do data-mixins because we won't see the data-mixins flag until too late.

So, it would require maintaining our own parser, and never registering d-treemap with document.register().

@ccmitchellusa
Copy link
Member

Could we define a d-mixin custom element to do the behavior mixin, rather than custom parsing of data attribute, eg.

<d-treemap data-mixins="">
   <d-mixin>dtreemap/Keyboard</d-mixin>
</d-treemap>

@wkeese
Copy link
Member Author

wkeese commented Oct 23, 2013

I'm not sure why we would do that. It seems more complicated.

@cjolif
Copy link
Contributor

cjolif commented Oct 24, 2013

The alternative is to ask users to redefine their own tag:

register("my-treemap", [TreeMap, Keyboard]);

and then use it:

<my-treemap></my-treemap>

I think this is very linked to #27. If we don't get auto-loading, then honesty this is probably not dramatic to ask people to do this as they will anyway have to manually import the TreeMap module. But if we keep the idea of being able to simply write markup and not have to deal with JavaScript to import/instantiate modules then having to fallback to JavaScript for the mixins won't fit the strategy.

In other words, if we don't implement #27 and close it I think this makes sense to be "consistent" and close also this one. If we implement #27 then I think we should do something for that one as well so that the pure markup solution is complete.

@cjolif
Copy link
Contributor

cjolif commented Nov 8, 2013

register("my-treemap", [TreeMap, Keyboard]);

won't work, apparently it needs to be:

register("my-treemap", [HTMLElement, TreeMap, Keyboard]);

despite TreeMap is already and HTMLElement.

Maybe we should have a helper function?

TreeMap.createSubWidget("my-treemap", [Keyboard]);

It should both include HTMLElement automatically and do the registration?

like we had in Dojo 1.X? Under that conditions it might be easier to accept the lack of data-dojo-mixins?

@wkeese
Copy link
Member Author

wkeese commented Nov 8, 2013

register("my-treemap", [TreeMap, Keyboard]);

is supposed to work. I guess there's a bug; I'll look into it.

@cjolif
Copy link
Contributor

cjolif commented Nov 8, 2013

Maybe there is something specific to the TreeMap that breaks this. Basically the widget is created but at runtime things like properties expected to be here are not and things like that.

@wkeese
Copy link
Member Author

wkeese commented Nov 26, 2013

For the record, I talked privately to @cjolif and he couldn't reproduce the problem with

register("my-treemap", [TreeMap, Keyboard]);

I did add some unit tests (in register.js) to make sure that sort of thing works.

@wkeese
Copy link
Member Author

wkeese commented Jul 18, 2014

For browsers that have native custom element support, you associate a prototype with a custom tag. I don't think it's practical or prudent to switch-out that prototype with a custom class we define on the fly.

So, if we did support data-mixin, the "classes" that we mix in would need to be lightweight objects. Perhaps a mixin would just be a function that would setting up advice on the main classes pre-existing methods.

@wkeese
Copy link
Member Author

wkeese commented Feb 2, 2015

if we did support data-mixin, the "classes" that we mix in would need to be lightweight objects. Perhaps a mixin would just be a function that would setting up advice on the main classes pre-existing methods.

Realistically I don't see us doing that (although it would be an interesting experiment). Thus I'm closing this ticket and also #27 for now.

@wkeese wkeese closed this as completed Feb 2, 2015
@wkeese wkeese added the wontfix label Feb 2, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants