Skip to content
This repository has been archived by the owner on Jul 30, 2018. It is now read-only.

Allow classes to be specified in the mark up for custom widgets #20

Closed
agubler opened this issue Jun 28, 2016 · 13 comments
Closed

Allow classes to be specified in the mark up for custom widgets #20

agubler opened this issue Jun 28, 2016 · 13 comments
Assignees
Milestone

Comments

@agubler
Copy link
Member

agubler commented Jun 28, 2016

As DOM nodes are discarded within widget-instances to add something like a container node you need to use custom element that factories the DOM node required, these are often static and stateless so there is little value having to manage an entry in a store using the stateFrom attribute in the data-options (provide classes etc).

There could simply be a classes attribute that is used to specify the classes on instantiation of the custom widget.

@agubler agubler changed the title Apply classes to be specified in the mark up for custom widgets Allow classes to be specified in the mark up for custom widgets Jun 28, 2016
@novemberborn
Copy link
Contributor

This would be sugar over data-options='{"classes":["foo","bar"]}' right?

Would it be data-classes? Split on /\s+/?

@agubler
Copy link
Member Author

agubler commented Jun 28, 2016

does data-options='{"classes":["foo","bar"]}' work?

@agubler
Copy link
Member Author

agubler commented Jun 28, 2016

<dojo-widget data-options='{"classes":["foo"]}'></dojo-widget> doesn't apply the classes to the rendered div dojo-widget uses createWidget as a factory

@novemberborn
Copy link
Contributor

The best the app factory can do is provide classes in the options object. So the widgets would have to be made to understand that option first. Then we could consider sugaring it like we've done for data-state-from.

@agubler
Copy link
Member Author

agubler commented Jun 28, 2016

If there is no id or stateFrom could the app factory use the default store to add a record with the options received in data-state-from?

@novemberborn
Copy link
Contributor

An ID would be required to use stores. Custom element factories do get their state from the app's default store if they have an ID.data-state-from contains a store identifier though, not options. Are you referring to data-state? Cause yes the factory patches that into the store (whichever store, really) before creating the widget instance. But again it needs an ID.

We could create a unique ID but that seems iffy. I'd rather explore taking classes as an option first.

@agubler
Copy link
Member Author

agubler commented Jun 28, 2016

yeah sorry data-state, okay adding an id would actually be okay as long as you can specify the state in the data-state attribute and there is a default store.

@novemberborn
Copy link
Contributor

To recap, currently you can pass classes as part of data-state. dojo/widgets#30 discusses specifying an initial value in widget options, which would allow it to be present in data-options. At that point we could lift it out of the options (much like listeners and stateFrom) so you could do data-classes, or classes: [] in a definition object.

@rishson
Copy link
Contributor

rishson commented Oct 4, 2016

@agubler status on this, still in-prog?

@novemberborn
Copy link
Contributor

Widgets can only take per-instance classes via their state. We can add data-classes (declarative) and classes: [] (in definition objects) and hoist it into the state object that's passed when creating the widget.

For consistency we should then disallow classes in data-state (declarative) and state definition objects.

@kitsonk
Copy link
Member

kitsonk commented Oct 19, 2016

For now, we think this is too dangerous and want to defer this until we have a better idea of why other ways of setting this are not workable.

@novemberborn
Copy link
Contributor

The "too dangerous" part referring to hoisting state properties out of data-state. Unlike those hosted out of data-options the state is not guaranteed to be used — it's initial state. By leaving the classes specification in data-state we make that relationship clear.

@dylans dylans modified the milestones: 2016.11, 2016.08 Oct 24, 2016
@dylans dylans modified the milestones: 2016.12, 2016.11 Nov 25, 2016
@dylans dylans modified the milestones: 2017.01, 2016.12 Dec 21, 2016
@dylans
Copy link
Member

dylans commented Jan 27, 2017

This is no longer relevant after the significant rework on widget-core (purpose of the ticket has been solved in other ways).

@dylans dylans closed this as completed Jan 27, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants