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

Universal IDs for widgets and other objects #9422

Open
johnspackman opened this Issue Nov 10, 2017 · 41 comments

Comments

Projects
None yet
6 participants
@johnspackman
Member

johnspackman commented Nov 10, 2017

It is useful to be able to identify widgets by an ID, whether for UI testing via a tool like Selenium or for general coding organisation. An example of Selenium testing (taken from a traditional web app test) is:

describe("Contact form", () => {
  test("lead can submit a contact request", async () => {
    await page.waitForSelector("[data-test=contact-form]");
    await page.click("input[name=name]");
    await page.type("input[name=name]", lead.name);
    await page.click("button[type=submit]");
    await page.waitForSelector(".modal");
  }, 16000);
});

Another use for IDs is during development as form of centralising widget creation in a form that can be overridden by derived classes - qx.ui.core.Widget has getChildControl and _createChildControlImpl which are specifically targeted at the construction of UI components, and the IDs are used for selecting appearance. However, this is targeted at the appearance mechanism and is very low level; it is not suitable for application level identification (and attempts to do so will break because of what the appearance mechanism expects to find there).

Parent/Child or Owner/Objects

There is benefit in having an ID mechanism for objects which is more application-centric; similar to getChildControl/_createChildControlImpl this would allow construction of objects to be centralised and allow derived classes to override features. Note that this should support any qx.core.Object and not just widgets.

At an application level, a particular form would have various fields (or other objects) that need to be identified, but the structure does not necessarily bear any relation to the widget layoutParent/children relationship. For example, consider a UserProfileEditor which has TextField's for personName and emailAddress, and a SelectBox for company; whether you place them all in one Composite with a Grid layout, or use two or three Composites with VBox/HBox layouts has nothing to do with the application-level structuring of the fields you want to identify.

To avoid confusion, we could call this application-level relationship Owner/Objects, where we add getObject and _createObjectImpl methods to some class in Qooxdoo (maybe qx.core.Object or as a mixin?)

Universal identification

By enforcing basic rules on names (eg alphanumeric plus "-" and "_"), we can assemble ID paths like "contactForm/edtName" and navigate the hierarchy from a given start point.

This would imply that every instance of a widget has an absolute path based identifier, which would be great for debugging or anything machine based.

How this would work with Selenium I'm not sure - AIUI Selenium would require a Sizzle expression to select widgets, which is fine for simple applications but would presumably become exponentially harder where there are dozens (or more) of forms in an application. Is it possible to add hooks into Selenium where we can add our own ID navigation?

@cboulanger

This comment has been minimized.

Show comment
Hide comment
@cboulanger

cboulanger Nov 10, 2017

Contributor

I don't know anything about Selenium, just getting my hands dirty with puppeteer - from which the above example comes. I completely agree with your approach - we should agree on a naming convention and support that within qooxdoo, and make it generic enough that different testing frameworks can interface with it.

One way would be to follow your Owner/Objects naming convention and then set a data-* attribute on the DOM node of the (parent/child) container widget. This way, it can be found by any testing framework.

Contributor

cboulanger commented Nov 10, 2017

I don't know anything about Selenium, just getting my hands dirty with puppeteer - from which the above example comes. I completely agree with your approach - we should agree on a naming convention and support that within qooxdoo, and make it generic enough that different testing frameworks can interface with it.

One way would be to follow your Owner/Objects naming convention and then set a data-* attribute on the DOM node of the (parent/child) container widget. This way, it can be found by any testing framework.

@cboulanger

This comment has been minimized.

Show comment
Hide comment
@cboulanger

cboulanger Nov 11, 2017

Contributor

I wonder what would be better - to have an arbitrary string as ID, which would, on the basis of a convention, be parsed into components to make it meaningful, such as "ownerId/objectID", or to enforce a structure with an API such as anyqxobject.set({ownerId:"ownerId", objectId:"objectId"}). The first option would be more flexible and would allow a 1:1 correspondence between a unique ID (= an absolute path) and its corresponding data-* attribute. Drawback would be that each target object would have a static ID. The second seems more portable, since it would allow parent-child relationships that could be moved between higher-level owner-objects (a relative path).

Contributor

cboulanger commented Nov 11, 2017

I wonder what would be better - to have an arbitrary string as ID, which would, on the basis of a convention, be parsed into components to make it meaningful, such as "ownerId/objectID", or to enforce a structure with an API such as anyqxobject.set({ownerId:"ownerId", objectId:"objectId"}). The first option would be more flexible and would allow a 1:1 correspondence between a unique ID (= an absolute path) and its corresponding data-* attribute. Drawback would be that each target object would have a static ID. The second seems more portable, since it would allow parent-child relationships that could be moved between higher-level owner-objects (a relative path).

@derrell

This comment has been minimized.

Show comment
Hide comment
@derrell

derrell Nov 11, 2017

Member

Is ownerId actually the parent's objectId? If not, then it might be useful to use the latter (map) capability, and add a parentId feature that would allow following the path up to figure out which particular widget is involved. (This would be very useful when inspecting the DOM and trying to figure out exactly what/where you're looking at.) I do realize this is diverging from the original intent of this issue...

Member

derrell commented Nov 11, 2017

Is ownerId actually the parent's objectId? If not, then it might be useful to use the latter (map) capability, and add a parentId feature that would allow following the path up to figure out which particular widget is involved. (This would be very useful when inspecting the DOM and trying to figure out exactly what/where you're looking at.) I do realize this is diverging from the original intent of this issue...

@cboulanger

This comment has been minimized.

Show comment
Hide comment
@cboulanger

cboulanger Nov 11, 2017

Contributor

On further reflection, the second option seems more flexible and versatile. Since the GUI test framework could use selectors to identify the widget's DOM node, ownerID and objectID could be separate data-* attributes. Using CSS selectors, one could then be able to get the DOM node by await page.$('[data-owner-id="foo"][data-object-id="bar"]'). One can even use XPATH with puppeteer for more complicated DOM queries.

Contributor

cboulanger commented Nov 11, 2017

On further reflection, the second option seems more flexible and versatile. Since the GUI test framework could use selectors to identify the widget's DOM node, ownerID and objectID could be separate data-* attributes. Using CSS selectors, one could then be able to get the DOM node by await page.$('[data-owner-id="foo"][data-object-id="bar"]'). One can even use XPATH with puppeteer for more complicated DOM queries.

@cboulanger

This comment has been minimized.

Show comment
Hide comment
@cboulanger

cboulanger Nov 11, 2017

Contributor

@derrell As I understand @johnspackman's proposal, ownerId is indeed the parent's objectId. qooxdoo would have to keep a registry which then can be queried to construct the parent/child path - This cries for a component which can inspect these relationships with a Tree widget :-)

Contributor

cboulanger commented Nov 11, 2017

@derrell As I understand @johnspackman's proposal, ownerId is indeed the parent's objectId. qooxdoo would have to keep a registry which then can be queried to construct the parent/child path - This cries for a component which can inspect these relationships with a Tree widget :-)

@derrell

This comment has been minimized.

Show comment
Hide comment
@derrell

derrell Nov 11, 2017

Member

Having an actual implementation is useful. Thanks! I see a few potential problems:

  • It's a PITA to add these all the time. Maybe we just need to live with it, if we want them, but it sure would be nice to find a way to automate the process.
  • More importantly, those IDs will get out of sync whenever you move code around. Automating the process, at least as far as setting the parentId (which I would, in fact, rename from ownerId). Whenever an object is added to a container, its parentId could be set automatically.
  • It currently depends on an "appear" event, but this should work with any qx.core.Object, not just with widgets, and in fact, should work with non-visible widgets as well

I may be viewing this from a different vantage point than others?

Member

derrell commented Nov 11, 2017

Having an actual implementation is useful. Thanks! I see a few potential problems:

  • It's a PITA to add these all the time. Maybe we just need to live with it, if we want them, but it sure would be nice to find a way to automate the process.
  • More importantly, those IDs will get out of sync whenever you move code around. Automating the process, at least as far as setting the parentId (which I would, in fact, rename from ownerId). Whenever an object is added to a container, its parentId could be set automatically.
  • It currently depends on an "appear" event, but this should work with any qx.core.Object, not just with widgets, and in fact, should work with non-visible widgets as well

I may be viewing this from a different vantage point than others?

@cboulanger

This comment has been minimized.

Show comment
Hide comment
@cboulanger

cboulanger Nov 11, 2017

Contributor

@derrell thank you for your comments! All very good points that should be addressed... One thing: we should probably automate this only in debug/test mode, because otherwise we needlessly create DOM properties...

Contributor

cboulanger commented Nov 11, 2017

@derrell thank you for your comments! All very good points that should be addressed... One thing: we should probably automate this only in debug/test mode, because otherwise we needlessly create DOM properties...

@cboulanger

This comment has been minimized.

Show comment
Hide comment
@cboulanger

cboulanger Nov 11, 2017

Contributor

What about a default object ID which is the class name with an incrementing instance count (such as qx.ui.window.Window[1] (qx.ui.window.Window[2], ...), which would allow paths like qx.ui.window.Window[1]/qx.ui.form.Button[1].

Maybe we could turn automatic object id assignment on and off through a flag, the default being off.

Contributor

cboulanger commented Nov 11, 2017

What about a default object ID which is the class name with an incrementing instance count (such as qx.ui.window.Window[1] (qx.ui.window.Window[2], ...), which would allow paths like qx.ui.window.Window[1]/qx.ui.form.Button[1].

Maybe we could turn automatic object id assignment on and off through a flag, the default being off.

@level420

This comment has been minimized.

Show comment
Hide comment
@level420

level420 Nov 11, 2017

Member

@cboulanger good idea! But it may be a better to not have dots and square brackets in the names as those tend to interfere with languages syntax like xpath etc. maybe just qx_ui_form_Button_1 or similar.

Member

level420 commented Nov 11, 2017

@cboulanger good idea! But it may be a better to not have dots and square brackets in the names as those tend to interfere with languages syntax like xpath etc. maybe just qx_ui_form_Button_1 or similar.

@level420

This comment has been minimized.

Show comment
Hide comment
@level420

level420 Nov 11, 2017

Member

and it would be really helpful to have the enumeration relative to the parent to allow adressing the first child of a parent like qx_ui_window_1/qx_ui_form_button_3 to find the third button instance in the first window instance.

Member

level420 commented Nov 11, 2017

and it would be really helpful to have the enumeration relative to the parent to allow adressing the first child of a parent like qx_ui_window_1/qx_ui_form_button_3 to find the third button instance in the first window instance.

@johnspackman

This comment has been minimized.

Show comment
Hide comment
@johnspackman

johnspackman Nov 11, 2017

Member

An ownerId will not work because it's not scalable and because it forces us back into a situation where there is a list of all objects in a map somewhere (i.e. remove automatic memory management); it's not scalable because it would imply that every ID was globally unique. My proposal is more that the Object would have a map of child object IDs to child objects, and each object would have an ID {String} and an Owner {qx.core.Object}.

The globally unique ID part would come because you could have an ID made up of paths, eg "/application/usersEditor/userProfileEditor/username" that could be navigated by a fairly simple iteration function.

I'm not convinced about relative IDs via a subscript (eg "Window[1]") either because it seems quite fragile - especially if it is arranged around UI components rather then logical/business components - what would happen if things were based around Composite[2], but that was subsequently changed from a Composite with a Grid layout to 3 Composites with VBox/HBox layouts? The unit tests should not have to change because of an implementation detail.

@derrell although I quite like the "parent/child" naming, that's kind of already taken with widgets and their children - although widget parents are called "layoutParent", children are "children" and so a noticably different naming could be really useful at times (I'm not tied to owner/owned, just that I think we should steer clear of "parent" and "child")

WRT how much of a PITA it is to assign individual IDs to widgets, I think it becomes more interesting if IDs are something that is assigned naturally - for example, if objects are created via calls to this.getObject("edtUsername") then they implicitly have an ID; this is the same pattern as the myWidget.getChildControl("icon"). From an application point of view, the reason you might start to use this pattern is that (a) it removes the spaghetti of ordering and member variables during form construction, and (b) it allows derived classes to replace objects in a controller manner.

Member

johnspackman commented Nov 11, 2017

An ownerId will not work because it's not scalable and because it forces us back into a situation where there is a list of all objects in a map somewhere (i.e. remove automatic memory management); it's not scalable because it would imply that every ID was globally unique. My proposal is more that the Object would have a map of child object IDs to child objects, and each object would have an ID {String} and an Owner {qx.core.Object}.

The globally unique ID part would come because you could have an ID made up of paths, eg "/application/usersEditor/userProfileEditor/username" that could be navigated by a fairly simple iteration function.

I'm not convinced about relative IDs via a subscript (eg "Window[1]") either because it seems quite fragile - especially if it is arranged around UI components rather then logical/business components - what would happen if things were based around Composite[2], but that was subsequently changed from a Composite with a Grid layout to 3 Composites with VBox/HBox layouts? The unit tests should not have to change because of an implementation detail.

@derrell although I quite like the "parent/child" naming, that's kind of already taken with widgets and their children - although widget parents are called "layoutParent", children are "children" and so a noticably different naming could be really useful at times (I'm not tied to owner/owned, just that I think we should steer clear of "parent" and "child")

WRT how much of a PITA it is to assign individual IDs to widgets, I think it becomes more interesting if IDs are something that is assigned naturally - for example, if objects are created via calls to this.getObject("edtUsername") then they implicitly have an ID; this is the same pattern as the myWidget.getChildControl("icon"). From an application point of view, the reason you might start to use this pattern is that (a) it removes the spaghetti of ordering and member variables during form construction, and (b) it allows derived classes to replace objects in a controller manner.

@johnspackman

This comment has been minimized.

Show comment
Hide comment
@johnspackman

johnspackman Nov 11, 2017

Member

PS my proposal does not cater well for existing applications, which I think adds a significant complication - the issue is that assigning an ID to a widget is easy, but making it possible to locate that widget via a globally unique equivalent is hard. In my proposal, IDs are unique because they are relative to their parent/owner, and this implies that the "owner" hierarchy is well defined. Existing applications will not have a regular "owner" hierarchy, or a means of navigating widgets in an application-centric manner, hence the need to use something like Window[2] which is so fragile.

Member

johnspackman commented Nov 11, 2017

PS my proposal does not cater well for existing applications, which I think adds a significant complication - the issue is that assigning an ID to a widget is easy, but making it possible to locate that widget via a globally unique equivalent is hard. In my proposal, IDs are unique because they are relative to their parent/owner, and this implies that the "owner" hierarchy is well defined. Existing applications will not have a regular "owner" hierarchy, or a means of navigating widgets in an application-centric manner, hence the need to use something like Window[2] which is so fragile.

@cboulanger

This comment has been minimized.

Show comment
Hide comment
@cboulanger

cboulanger Nov 11, 2017

Contributor

Can't we take up the "control" vocabulary - so inside widgets, we have "child controls", inside an application we have controls and "subcontrols". This would allow us to create control hierarchies that reach down to the child controls, such as "/editor/sendButton/label". Assigning the data-* attributes for child controls can be done automatically since they already have an ID.

Contributor

cboulanger commented Nov 11, 2017

Can't we take up the "control" vocabulary - so inside widgets, we have "child controls", inside an application we have controls and "subcontrols". This would allow us to create control hierarchies that reach down to the child controls, such as "/editor/sendButton/label". Assigning the data-* attributes for child controls can be done automatically since they already have an ID.

@cboulanger

This comment has been minimized.

Show comment
Hide comment
@cboulanger

cboulanger Nov 11, 2017

Contributor

I just wonder how, without a central registry, you can find, given a path, the top "controls/parents" which allow you to drill into their "subcontrol/children" chain?

Contributor

cboulanger commented Nov 11, 2017

I just wonder how, without a central registry, you can find, given a path, the top "controls/parents" which allow you to drill into their "subcontrol/children" chain?

@johnspackman

This comment has been minimized.

Show comment
Hide comment
@johnspackman

johnspackman Nov 12, 2017

Member

"Control" implies a UI control so it isn't ideal because it's often more than controls, eg the pattern for creating objects by ID is useful for creating qx.data.controller.* and other common objects.

To locate something without a registry, you just have to have a starting point; that could be some global object which registers qx.core.Init.getApplication() as (eg) "application" so that paths would be "/application/this/that/usersEditor/userEditor/name".

Relative paths would be the same so something like:

var usersEditor = qx.core.Ids.getObject("/application/this/that/usersEditor");
var txtUserName = usersEditor.getObject("userEditor/name");
Member

johnspackman commented Nov 12, 2017

"Control" implies a UI control so it isn't ideal because it's often more than controls, eg the pattern for creating objects by ID is useful for creating qx.data.controller.* and other common objects.

To locate something without a registry, you just have to have a starting point; that could be some global object which registers qx.core.Init.getApplication() as (eg) "application" so that paths would be "/application/this/that/usersEditor/userEditor/name".

Relative paths would be the same so something like:

var usersEditor = qx.core.Ids.getObject("/application/this/that/usersEditor");
var txtUserName = usersEditor.getObject("userEditor/name");
@cboulanger

This comment has been minimized.

Show comment
Hide comment
@cboulanger

cboulanger Nov 12, 2017

Contributor

Sounds great to me. Very simple API. I have a bias towards UI controls because I am interested in GUI tests. But you are totally right that this needs to be a more generic system.

One more thought: I wonder if we should design the system so that it could store some meta-data about the structure of the application that could be visualized using the API viewer, for example, an extra tab on the left "app structure" and that could be exported to allow third parties (such as plugin developers) to hook into the app. This is a problem I have been trying to solve with my app forever, using all kind of hacks, and would love to see as an official qooxdoo feature.

Use case: a plugin provides an additional menu item in the main application, or replaces the textfield widget in a modal window with an HTML editor.

Contributor

cboulanger commented Nov 12, 2017

Sounds great to me. Very simple API. I have a bias towards UI controls because I am interested in GUI tests. But you are totally right that this needs to be a more generic system.

One more thought: I wonder if we should design the system so that it could store some meta-data about the structure of the application that could be visualized using the API viewer, for example, an extra tab on the left "app structure" and that could be exported to allow third parties (such as plugin developers) to hook into the app. This is a problem I have been trying to solve with my app forever, using all kind of hacks, and would love to see as an official qooxdoo feature.

Use case: a plugin provides an additional menu item in the main application, or replaces the textfield widget in a modal window with an HTML editor.

@johnspackman

This comment has been minimized.

Show comment
Hide comment
@johnspackman

johnspackman Nov 12, 2017

Member

I wrote up a quick gist of what I'm imagining, based on your MObjectId but not tested, just written up: https://gist.github.com/johnspackman/f109bae1756e6f19f30c429c007f9c95

Re application structure: yes, that could be really really cool and I think that this ID mechanism would enable it; there was/is an inspector somewhere that you could add in to browse your app (at the Widget level). AFAICR it was a bit too low level and not straightforward to get hooked in so I never used it. There are so many times when, for example, you want to know what is in the model of the qx.data.controller.List that populates a certain SelectBox but you're not in the debugger to be able to get a pointer to it.

Member

johnspackman commented Nov 12, 2017

I wrote up a quick gist of what I'm imagining, based on your MObjectId but not tested, just written up: https://gist.github.com/johnspackman/f109bae1756e6f19f30c429c007f9c95

Re application structure: yes, that could be really really cool and I think that this ID mechanism would enable it; there was/is an inspector somewhere that you could add in to browse your app (at the Widget level). AFAICR it was a bit too low level and not straightforward to get hooked in so I never used it. There are so many times when, for example, you want to know what is in the model of the qx.data.controller.List that populates a certain SelectBox but you're not in the debugger to be able to get a pointer to it.

@cboulanger

This comment has been minimized.

Show comment
Hide comment
@cboulanger

cboulanger Nov 12, 2017

Contributor

Simple and elegant. I am in favour of this approach and would support a PR in this direction. The meta-data/documentation I was thinking of could be stored in the comments, I guess that the ID is all we need in the code itself. I am thinking of some field that would map ids to "descriptions" that explain what the object does in terms of application/business logic (not as a class, but as a feature).

Contributor

cboulanger commented Nov 12, 2017

Simple and elegant. I am in favour of this approach and would support a PR in this direction. The meta-data/documentation I was thinking of could be stored in the comments, I guess that the ID is all we need in the code itself. I am thinking of some field that would map ids to "descriptions" that explain what the object does in terms of application/business logic (not as a class, but as a feature).

@cboulanger

This comment has been minimized.

Show comment
Hide comment
@cboulanger

cboulanger Nov 12, 2017

Contributor

Maybe the automated id assignment that @cajus and @derrell would like to see could be implemented by including separate "homemade" mixins that would allow to use their already established patterns? I think the mechanism we provide on the framework level should be as open and generic as possible and avoid to determine a specific way of automatically assigning IDs. Then we can experiment and maybe later include a pattern that has proven to be useful.

Contributor

cboulanger commented Nov 12, 2017

Maybe the automated id assignment that @cajus and @derrell would like to see could be implemented by including separate "homemade" mixins that would allow to use their already established patterns? I think the mechanism we provide on the framework level should be as open and generic as possible and avoid to determine a specific way of automatically assigning IDs. Then we can experiment and maybe later include a pattern that has proven to be useful.

@johnspackman

This comment has been minimized.

Show comment
Hide comment
@johnspackman

johnspackman Nov 12, 2017

Member

ah, ISWYM - a great way to do that would be to use annotations because it is an existing mechanism that would save us from having to define what meta data could to be available. The ID mechanism is simply a way to navigate the structure, so if you added an annotation specific to libraries or researchers in bibliograph it would be up to some other tool (eg an "Inspector" widget) to decide how to interpret that meta data. Similarly, adding a generic "description" annotation is just about defining that annotation and having classes reference it.

Member

johnspackman commented Nov 12, 2017

ah, ISWYM - a great way to do that would be to use annotations because it is an existing mechanism that would save us from having to define what meta data could to be available. The ID mechanism is simply a way to navigate the structure, so if you added an annotation specific to libraries or researchers in bibliograph it would be up to some other tool (eg an "Inspector" widget) to decide how to interpret that meta data. Similarly, adding a generic "description" annotation is just about defining that annotation and having classes reference it.

@johnspackman

This comment has been minimized.

Show comment
Hide comment
@johnspackman

johnspackman Nov 12, 2017

Member

WRT automating the process, once IDs are assigned I think most of the automation is already taken care of - I updated that gist just now to fix the missing code for this, but when you call this.getObject("txtName") and it calls this._createObjectImpl, the returned object has obj.setOwner(this) passed to it.

Also, by calling widget.getContentElement().setAttribute(...) the IDs are automatically added and removed as the widget gets bound to the DOM.

What is left is assigning a globally unique ID to each DOM element, which could be expensive unless it is carefully cached - i.e. translating the username text field's ID of "name" to "_application_this_that_usersEditor_userEditor_name". I imagine that this will be pretty essential to use with selenium, unless there is a way of hooking into Selenium's selection mechanism and adding a plugin which can interpret paths and recurse through the DOM.

Member

johnspackman commented Nov 12, 2017

WRT automating the process, once IDs are assigned I think most of the automation is already taken care of - I updated that gist just now to fix the missing code for this, but when you call this.getObject("txtName") and it calls this._createObjectImpl, the returned object has obj.setOwner(this) passed to it.

Also, by calling widget.getContentElement().setAttribute(...) the IDs are automatically added and removed as the widget gets bound to the DOM.

What is left is assigning a globally unique ID to each DOM element, which could be expensive unless it is carefully cached - i.e. translating the username text field's ID of "name" to "_application_this_that_usersEditor_userEditor_name". I imagine that this will be pretty essential to use with selenium, unless there is a way of hooking into Selenium's selection mechanism and adding a plugin which can interpret paths and recurse through the DOM.

@cboulanger

This comment has been minimized.

Show comment
Hide comment
@cboulanger

cboulanger Nov 12, 2017

Contributor

I think what they were talking about was to use autogenerated object ids (based on the class name, some increment, and so on.) I also think that could be useful but I would argue for not making this a basic feature of the mechanism because my sense was that there was no consensus about the syntax of how this should be implemented. Rather, we should have implementations based on your solution, and then we can pick and choose which one proves to be most useful.

Contributor

cboulanger commented Nov 12, 2017

I think what they were talking about was to use autogenerated object ids (based on the class name, some increment, and so on.) I also think that could be useful but I would argue for not making this a basic feature of the mechanism because my sense was that there was no consensus about the syntax of how this should be implemented. Rather, we should have implementations based on your solution, and then we can pick and choose which one proves to be most useful.

@johnspackman

This comment has been minimized.

Show comment
Hide comment
@johnspackman

johnspackman Nov 12, 2017

Member

Oh, so the "qx.core.window.Window[2]/qx.ui.form.TextField[3]" form? That sounds more like an expression language, but also sounds very fragile (eg minor visual/layout changes break unit tests)

Perhaps we could do something like, first register the window globally (eg as "userProfileEditor") so it can be found, and then assign IDs to each created widget (eg "edtUsername" TextField). In the window's .add() method we could detect that the widget has an ID but does not have an owner and so then hook it up to the window. This would mean that IDs could be easily retrofitted into an application (which has to be done anyway, and as it's just one more {String} property to set the work would be as simple as it could be), and provided that owned widget is directly added to the window everything will work as expected.

There would be a final piece of the puzzle where fields were added to irrelevant intermediate widgets (like a groupbox) that when it comes to IDs and paths you actually want to ignore completely; so to do this, we'd have a registerObject(obj...) API method that would be able to manually attach the widgets directly to the window.

EDIT: The end result would be that "/userProfileEditor/edtUsername" would be the absolute path for the widget and could be translated into a DOM as <div id="_userProfileEditor_edtUsername">

Member

johnspackman commented Nov 12, 2017

Oh, so the "qx.core.window.Window[2]/qx.ui.form.TextField[3]" form? That sounds more like an expression language, but also sounds very fragile (eg minor visual/layout changes break unit tests)

Perhaps we could do something like, first register the window globally (eg as "userProfileEditor") so it can be found, and then assign IDs to each created widget (eg "edtUsername" TextField). In the window's .add() method we could detect that the widget has an ID but does not have an owner and so then hook it up to the window. This would mean that IDs could be easily retrofitted into an application (which has to be done anyway, and as it's just one more {String} property to set the work would be as simple as it could be), and provided that owned widget is directly added to the window everything will work as expected.

There would be a final piece of the puzzle where fields were added to irrelevant intermediate widgets (like a groupbox) that when it comes to IDs and paths you actually want to ignore completely; so to do this, we'd have a registerObject(obj...) API method that would be able to manually attach the widgets directly to the window.

EDIT: The end result would be that "/userProfileEditor/edtUsername" would be the absolute path for the widget and could be translated into a DOM as <div id="_userProfileEditor_edtUsername">

@cboulanger

This comment has been minimized.

Show comment
Hide comment
@cboulanger

cboulanger Nov 12, 2017

Contributor

Automatic Id assignment could also draw from the text of titles, labels etc., but as I said, I would prefer to have this as a separate solution that can be added in on demand.

Contributor

cboulanger commented Nov 12, 2017

Automatic Id assignment could also draw from the text of titles, labels etc., but as I said, I would prefer to have this as a separate solution that can be added in on demand.

@johnspackman

This comment has been minimized.

Show comment
Hide comment
@johnspackman

johnspackman Nov 12, 2017

Member

Interesting - I don't use forms, but is it typical to associate a label with a form field? i.e. in the way that a form field can discover it's own label?

Member

johnspackman commented Nov 12, 2017

Interesting - I don't use forms, but is it typical to associate a label with a form field? i.e. in the way that a form field can discover it's own label?

@cboulanger

This comment has been minimized.

Show comment
Hide comment
@cboulanger

cboulanger Nov 12, 2017

Contributor

You're right - I haven't worked on my forms for a while. Contrary to what I thought, there is no way to associate labels and forms as there is in HTML with the "label for" mechanism. Maybe there should be.

Contributor

cboulanger commented Nov 12, 2017

You're right - I haven't worked on my forms for a while. Contrary to what I thought, there is no way to associate labels and forms as there is in HTML with the "label for" mechanism. Maybe there should be.

@level420

This comment has been minimized.

Show comment
Hide comment
@level420

level420 Nov 12, 2017

Member

If you are using qx.ui.form.Form.add you are adding form fields including their label string values and a field name. The form renderer then creates label widget instances and associates them with the form field by e.g. binding their enabled property so that if you disable the field it also sets the label disabled. I’m using forms a lot.

Member

level420 commented Nov 12, 2017

If you are using qx.ui.form.Form.add you are adding form fields including their label string values and a field name. The form renderer then creates label widget instances and associates them with the form field by e.g. binding their enabled property so that if you disable the field it also sets the label disabled. I’m using forms a lot.

@cboulanger

This comment has been minimized.

Show comment
Hide comment
@cboulanger

cboulanger Nov 12, 2017

Contributor

qx.ui.form.CheckBox has a label (as a subclass of qx.ui.basic.Atom), of course, but few other form widgets. Maybe this could be implemented in qx.ui.core.MForm.

Contributor

cboulanger commented Nov 12, 2017

qx.ui.form.CheckBox has a label (as a subclass of qx.ui.basic.Atom), of course, but few other form widgets. Maybe this could be implemented in qx.ui.core.MForm.

@cboulanger

This comment has been minimized.

Show comment
Hide comment
@cboulanger

cboulanger Nov 12, 2017

Contributor

@level420 of course! the form has the information on fieldnames and labels. Our mechanism could tap into this!

Contributor

cboulanger commented Nov 12, 2017

@level420 of course! the form has the information on fieldnames and labels. Our mechanism could tap into this!

@level420

This comment has been minimized.

Show comment
Hide comment
@level420

level420 Nov 12, 2017

Member

In fact I’m using a derived form class where I’ve extended the interface to be able to retreive fields by their name. If I’ve added a field ‘username’, I’m able to get itbfrom the form instance via fom.getItem(‘username’).

Member

level420 commented Nov 12, 2017

In fact I’m using a derived form class where I’ve extended the interface to be able to retreive fields by their name. If I’ve added a field ‘username’, I’m able to get itbfrom the form instance via fom.getItem(‘username’).

@cboulanger

This comment has been minimized.

Show comment
Hide comment
@cboulanger

cboulanger Nov 12, 2017

Contributor

Qooxdoo provides a way to bind model properties to form fields (see qx.data.controller.Form)- so that the property names become the field names/ids. But it is difficult to auto-discover this because the field widgets know nothing about this controller, and the controller is not in some parent-child relationship to them. Unfortunately, this is more complex than I initiatially thought and probably beyond our new mechanism.

Contributor

cboulanger commented Nov 12, 2017

Qooxdoo provides a way to bind model properties to form fields (see qx.data.controller.Form)- so that the property names become the field names/ids. But it is difficult to auto-discover this because the field widgets know nothing about this controller, and the controller is not in some parent-child relationship to them. Unfortunately, this is more complex than I initiatially thought and probably beyond our new mechanism.

@johnspackman

This comment has been minimized.

Show comment
Hide comment
@johnspackman

johnspackman Nov 12, 2017

Member

It would be better to find a way to handle auto ids via Form rather than trying to hook into 'add' and 'remove' because that would impact a lot of widgets and require some careful coding to prevent conflicts with how ids are handled normally.

Member

johnspackman commented Nov 12, 2017

It would be better to find a way to handle auto ids via Form rather than trying to hook into 'add' and 'remove' because that would impact a lot of widgets and require some careful coding to prevent conflicts with how ids are handled normally.

@derrell

This comment has been minimized.

Show comment
Hide comment
@derrell

derrell Nov 12, 2017

Member

I think I may not have been clear. My goal is that when something goes wrong in my app, to be able to look at the DOM and quickly and easily ascertain how (where from) that DOM element was created. Currently, I have to intuit even what type of widget it is from the class value, and then go searching through code to figure out what the possibly many widgets are that use that class. I don't (for this purpose) case about a registry at all. All I care about is that there is an ID that is meaningful, preferably on its own, but if necessary (because of child controls or widgets created by widgets), by following its parent path to ascertain which widget my program instantiated that's having the problem.

I don't mind if I have to register the code that generates the IDs, as long as they are created automatically and are there when I discover I need them. That code that I register would presumably be some function that's passed lots of info about whose ID is being generated, so that I can build an appropriate ID.

Member

derrell commented Nov 12, 2017

I think I may not have been clear. My goal is that when something goes wrong in my app, to be able to look at the DOM and quickly and easily ascertain how (where from) that DOM element was created. Currently, I have to intuit even what type of widget it is from the class value, and then go searching through code to figure out what the possibly many widgets are that use that class. I don't (for this purpose) case about a registry at all. All I care about is that there is an ID that is meaningful, preferably on its own, but if necessary (because of child controls or widgets created by widgets), by following its parent path to ascertain which widget my program instantiated that's having the problem.

I don't mind if I have to register the code that generates the IDs, as long as they are created automatically and are there when I discover I need them. That code that I register would presumably be some function that's passed lots of info about whose ID is being generated, so that I can build an appropriate ID.

@petitben

This comment has been minimized.

Show comment
Hide comment
@petitben

petitben Nov 17, 2017

Contributor

Very interesting topic. I looked for this kind of feature for many years :-)

I know Extjs provides this kind of mechanism using Ext.ComponentQuery. Actually it mimics the functionalities from the standard querySelector. It might worth a look at it for inspiration purpose.

Anyway I wonder if base the work on qx.core.ObjectRegistry would do the trick (as every object can be identified using toHashCode() and toString()).

Voilà, just my 2 cents :-)

Contributor

petitben commented Nov 17, 2017

Very interesting topic. I looked for this kind of feature for many years :-)

I know Extjs provides this kind of mechanism using Ext.ComponentQuery. Actually it mimics the functionalities from the standard querySelector. It might worth a look at it for inspiration purpose.

Anyway I wonder if base the work on qx.core.ObjectRegistry would do the trick (as every object can be identified using toHashCode() and toString()).

Voilà, just my 2 cents :-)

@johnspackman

This comment has been minimized.

Show comment
Hide comment
@johnspackman

johnspackman Nov 17, 2017

Member

I've never used ExtJS but a quick google and the query mechanism looks interesting; this ID mechanism would be a foundation piece for such a query language, but we should probably consider that as a separate PR

The disadvantage of ObjectRegistry is that having a central list of all objects makes memory management a nightmare, especially for incidental objects - there's a writeup here on the changes we made to remove the reliance on the registry. The hashcodes are also opaque and not guaranteed repeatable, whereas the ID mechanism here is designed to be human readable (all objects continue to have a hash code though, it's just that they are not registered anywhere by default)

This will be coming up as a PR "real soon now" :)

Member

johnspackman commented Nov 17, 2017

I've never used ExtJS but a quick google and the query mechanism looks interesting; this ID mechanism would be a foundation piece for such a query language, but we should probably consider that as a separate PR

The disadvantage of ObjectRegistry is that having a central list of all objects makes memory management a nightmare, especially for incidental objects - there's a writeup here on the changes we made to remove the reliance on the registry. The hashcodes are also opaque and not guaranteed repeatable, whereas the ID mechanism here is designed to be human readable (all objects continue to have a hash code though, it's just that they are not registered anywhere by default)

This will be coming up as a PR "real soon now" :)

johnspackman added a commit to johnspackman/qooxdoo that referenced this issue Nov 17, 2017

johnspackman added a commit to johnspackman/qooxdoo that referenced this issue Nov 17, 2017

Merge branch 'object-ids'
* object-ids:
  lint & better comments
  Provides Universal Widget ID implementation discussed in qooxdoo#9422

johnspackman added a commit to johnspackman/qooxdoo that referenced this issue Nov 17, 2017

@johnspackman

This comment has been minimized.

Show comment
Hide comment
@johnspackman

johnspackman Nov 22, 2017

Member

This ID mechanism isn't just good for testing and pre-production, it's also very useful for development - when writing an object which has a lot of controls or other objects (eg you're writing a form of some kind such as an editor widget or a dialog box), your constructor might look like this:

construct: function() {
  this.base(arguments);
  var comp = new qx.ui.container.Composite();
  var edt = new qx.form.TextField();
  this._addToGroup(comp, edt, "Line 1", "line1"); 

  edt = new qx.form.TextField();
  this._addToGroup(comp, edt, "Line 2", "line2"); 
  /* ... snip ... */

  edt = new qx.form.TextField();
  this._addToGroup(comp, edt, "Postcode", "postcode"); 
  // ... etc ...
},
members: {
  _addToGroup: function(parentWidget, widget, label, bindPath) {
    /* ... snip ... */
  }
}

As forms get more complex, so does the constructor; adding event listeners and other behaviour increase the size of the constructor and make it harder to read what exactly is going on. And as layout is redesigned, the order of the code in constructor becomes important because obviously you cannot register event listeners on objects that have not been created yet.

When those widgets need to be referenced by event handlers or other methods, you need to make sure that each one has a unique name, and/or is assigned to a unique member variable too.

IMHO this rapidly makes the constructor become a big mixup of UI code, layout code, and normal construction/setup code. For example:

construct: function() {
  this.base(arguments);
  var comp = new qx.ui.container.Composite();
  var edtLine1 = this.__edtLine1 = new qx.form.TextField();
  this._addToGroup(comp, edtLine1, "Line 1", "line1"); 

  var edtLine2 = this.__edtLine2 = new qx.form.TextField();
  this._addToGroup(comp, edtLine2, "Line 2", "line2"); 
  /* ... snip ... */

  var edtPostcode = this.__edtPostcode = new qx.form.TextField();
  this._addToGroup(comp, edtPostcode, "Postcode", "postcode"); 
  edtPostCode.addListener("changeValue", (evt) => {
    var str = evt.getData();
    var address = myapp.PostcodeValidator.lookup(str);
    if (address) {
      edtCountry.setValue(address.getCountry());
    }
  });
  // ... etc ...
}

As forms get larger it helps to break this out into a clearer style. Here's a real world example which uses the ID mechanism to achieve that:

qx.Class.define("myapp.AddressEditor", {
  extend: myapp.Editor,

  construct: function() {
    // The base class will know how to create the UI because it is being told
    //   to create the "comp" widget on demand
    this.base(arguments, null, [ "comp" ]);
  },

  members: {
    _applyParentValue: function(value, oldValue) {
      this.base(arguments, value, oldValue);
      // Access control is applied to objects by name
      this._setAccessControl(value, ["edtLine1", "edtLine2", "edtLine3", "edtCity", "edtCounty", "edtPostcode", 
        "edtCountry", "edtTelephone", "edtFax", "edtEmail" ]);
    },

    _createObjectImpl: function(id) {
      switch(id) {
      case "comp":
        var comp = new qx.ui.container.Composite();
        this._addToGroup(comp, "edtLine1", "Line 1", null, "line1");
        this._addToGroup(comp, "edtLine2", "Line 2", null, "line2");
        this._addToGroup(comp, "edtLine3", "Line 3", null, "line3");
        this._addToGroup(comp, "edtCity", "City", null, "city");
        this._addToGroup(comp, "edtCounty", "County", null, "county");
        this._addToGroup(comp, "edtPostcode", "Postcode", null, "postcode");
        this._addToGroup(comp, "edtCountry", "Country", null, "country");
        this._addToGroup(comp, "edtTelephone", "Main Telephone", null, "telephone");
        this._addToGroup(comp, "edtMobile", "Mobile/Alt Telephone", null, "mobile");
        this._addToGroup(comp, "edtFax", "Main Fax", null, "fax");
        this._addToGroup(comp, "edtEmail", "General Email", null, "email");
        var lay = comp.getLayout();
        lay.set({ spacingY: 3 });
        return comp;

      case "edtLine1":
      case "edtLine2":
      case "edtLine3":
      case "edtCity":
      case "edtCounty":
        return new qx.ui.form.TextField();

      case "edtPostcode":
        var edt = new qx.ui.form.TextField().set({ maxWidth: 150 });
        edtPostCode.addListener("changeValue", (evt) => {
        var str = evt.getData();
        var address = myapp.PostcodeValidator.lookup(str);
        if (address) {
          edtCountry.setValue(address.getCountry());
        }
        return edt;

      case "edtCountry":
        return new qx.ui.form.TextField().set({ maxWidth: 150 });

      case "edtTelephone":
      case "edtMobile":
      case "edtFax":
        return new qx.ui.form.TextField().set({ maxWidth: 150 });

      case "edtEmail":
        return new qx.ui.form.TextField();
      }

      return this.base(arguments, id);
    }
  }
});

Every widget becomes a member of the class, and although in this example they are all "public" there is nothing to prevent you using "_" or "__" prefixes to denote protected/private members. It's easy and natural to pass around ID strings instead of object instances, and it means that if you derive a class from this, the derived class can change the widget implementation completely just by overriding _createObjectImpl.

Personally, I find that the visual impact when reading code is really great because I can see at a glance where widgets are and the constructor is much less cluttered. Syntax highlighting in my editor means that the strings in case "edtLine1": lines are highlighted and draw your eye to the different controls at least as good as comments (without requiring an additional comment).

Member

johnspackman commented Nov 22, 2017

This ID mechanism isn't just good for testing and pre-production, it's also very useful for development - when writing an object which has a lot of controls or other objects (eg you're writing a form of some kind such as an editor widget or a dialog box), your constructor might look like this:

construct: function() {
  this.base(arguments);
  var comp = new qx.ui.container.Composite();
  var edt = new qx.form.TextField();
  this._addToGroup(comp, edt, "Line 1", "line1"); 

  edt = new qx.form.TextField();
  this._addToGroup(comp, edt, "Line 2", "line2"); 
  /* ... snip ... */

  edt = new qx.form.TextField();
  this._addToGroup(comp, edt, "Postcode", "postcode"); 
  // ... etc ...
},
members: {
  _addToGroup: function(parentWidget, widget, label, bindPath) {
    /* ... snip ... */
  }
}

As forms get more complex, so does the constructor; adding event listeners and other behaviour increase the size of the constructor and make it harder to read what exactly is going on. And as layout is redesigned, the order of the code in constructor becomes important because obviously you cannot register event listeners on objects that have not been created yet.

When those widgets need to be referenced by event handlers or other methods, you need to make sure that each one has a unique name, and/or is assigned to a unique member variable too.

IMHO this rapidly makes the constructor become a big mixup of UI code, layout code, and normal construction/setup code. For example:

construct: function() {
  this.base(arguments);
  var comp = new qx.ui.container.Composite();
  var edtLine1 = this.__edtLine1 = new qx.form.TextField();
  this._addToGroup(comp, edtLine1, "Line 1", "line1"); 

  var edtLine2 = this.__edtLine2 = new qx.form.TextField();
  this._addToGroup(comp, edtLine2, "Line 2", "line2"); 
  /* ... snip ... */

  var edtPostcode = this.__edtPostcode = new qx.form.TextField();
  this._addToGroup(comp, edtPostcode, "Postcode", "postcode"); 
  edtPostCode.addListener("changeValue", (evt) => {
    var str = evt.getData();
    var address = myapp.PostcodeValidator.lookup(str);
    if (address) {
      edtCountry.setValue(address.getCountry());
    }
  });
  // ... etc ...
}

As forms get larger it helps to break this out into a clearer style. Here's a real world example which uses the ID mechanism to achieve that:

qx.Class.define("myapp.AddressEditor", {
  extend: myapp.Editor,

  construct: function() {
    // The base class will know how to create the UI because it is being told
    //   to create the "comp" widget on demand
    this.base(arguments, null, [ "comp" ]);
  },

  members: {
    _applyParentValue: function(value, oldValue) {
      this.base(arguments, value, oldValue);
      // Access control is applied to objects by name
      this._setAccessControl(value, ["edtLine1", "edtLine2", "edtLine3", "edtCity", "edtCounty", "edtPostcode", 
        "edtCountry", "edtTelephone", "edtFax", "edtEmail" ]);
    },

    _createObjectImpl: function(id) {
      switch(id) {
      case "comp":
        var comp = new qx.ui.container.Composite();
        this._addToGroup(comp, "edtLine1", "Line 1", null, "line1");
        this._addToGroup(comp, "edtLine2", "Line 2", null, "line2");
        this._addToGroup(comp, "edtLine3", "Line 3", null, "line3");
        this._addToGroup(comp, "edtCity", "City", null, "city");
        this._addToGroup(comp, "edtCounty", "County", null, "county");
        this._addToGroup(comp, "edtPostcode", "Postcode", null, "postcode");
        this._addToGroup(comp, "edtCountry", "Country", null, "country");
        this._addToGroup(comp, "edtTelephone", "Main Telephone", null, "telephone");
        this._addToGroup(comp, "edtMobile", "Mobile/Alt Telephone", null, "mobile");
        this._addToGroup(comp, "edtFax", "Main Fax", null, "fax");
        this._addToGroup(comp, "edtEmail", "General Email", null, "email");
        var lay = comp.getLayout();
        lay.set({ spacingY: 3 });
        return comp;

      case "edtLine1":
      case "edtLine2":
      case "edtLine3":
      case "edtCity":
      case "edtCounty":
        return new qx.ui.form.TextField();

      case "edtPostcode":
        var edt = new qx.ui.form.TextField().set({ maxWidth: 150 });
        edtPostCode.addListener("changeValue", (evt) => {
        var str = evt.getData();
        var address = myapp.PostcodeValidator.lookup(str);
        if (address) {
          edtCountry.setValue(address.getCountry());
        }
        return edt;

      case "edtCountry":
        return new qx.ui.form.TextField().set({ maxWidth: 150 });

      case "edtTelephone":
      case "edtMobile":
      case "edtFax":
        return new qx.ui.form.TextField().set({ maxWidth: 150 });

      case "edtEmail":
        return new qx.ui.form.TextField();
      }

      return this.base(arguments, id);
    }
  }
});

Every widget becomes a member of the class, and although in this example they are all "public" there is nothing to prevent you using "_" or "__" prefixes to denote protected/private members. It's easy and natural to pass around ID strings instead of object instances, and it means that if you derive a class from this, the derived class can change the widget implementation completely just by overriding _createObjectImpl.

Personally, I find that the visual impact when reading code is really great because I can see at a glance where widgets are and the constructor is much less cluttered. Syntax highlighting in my editor means that the strings in case "edtLine1": lines are highlighted and draw your eye to the different controls at least as good as comments (without requiring an additional comment).

@cajus

This comment has been minimized.

Show comment
Hide comment
@cajus

cajus Nov 22, 2017

Contributor

Hmm. I don't understand how your example works. Mustn't it at least call the base _createObjectImpl to make the objects beeing registered?

More thoughts: That mechanism isn't valid if you're using _createChildControlImpl most of the time to control widget styling. Setting up a form is just a small part of an application, and we even use _createChildControlImpl for that here, because it also handles destruction for us. The _createObjectImpl code keeps references to the objects that are not removed in the destructor. I don't know whether the automatic memory management will help there, but we had massive problems with non disposed widgets, controllers, etc. in the past that have not been disposed in the destructor. Partly even the destruction order is important if you don't want to run into crashes (mostly list/controller).

Why not just using the _createChildControlImpl mechanism for that, too? It's feels much more natural if you've large widget structures IMHO.

Contributor

cajus commented Nov 22, 2017

Hmm. I don't understand how your example works. Mustn't it at least call the base _createObjectImpl to make the objects beeing registered?

More thoughts: That mechanism isn't valid if you're using _createChildControlImpl most of the time to control widget styling. Setting up a form is just a small part of an application, and we even use _createChildControlImpl for that here, because it also handles destruction for us. The _createObjectImpl code keeps references to the objects that are not removed in the destructor. I don't know whether the automatic memory management will help there, but we had massive problems with non disposed widgets, controllers, etc. in the past that have not been disposed in the destructor. Partly even the destruction order is important if you don't want to run into crashes (mostly list/controller).

Why not just using the _createChildControlImpl mechanism for that, too? It's feels much more natural if you've large widget structures IMHO.

@johnspackman

This comment has been minimized.

Show comment
Hide comment
@johnspackman

johnspackman Nov 22, 2017

Member

Hmm. I don't understand how your example works. Mustn't it at least call the base _createObjectImpl to make the objects beeing registered?

It should, you're right - but not to make the objects become registered (calling base would only be necessary if the base class might create widgets on demand also). When you need to access an object, you call getObject("edtPostcode") and then will only call _createObjectImpl the first time, to create the object - after that it is cached. This is the same pattern as getChildControl/_createChildControlImpl

More thoughts: That mechanism isn't valid if you're using _createChildControlImpl most of the time to control widget styling. Setting up a form is just a small part of an application, and we even use _createChildControlImpl for that here
Fair point, my mixin here will defer to getChildControl if _createObjectImpl returned undefined.

Why not just using the _createChildControlImpl mechanism for that, too? It's feels much more natural if you've large widget structures IMHO.

That was my first attempt years ago but found that there were conflicts with getting the appearances to work, and it might require that everything returned is a LayoutItem or Widget. It's a long time ago now and maybe things have moved on, but at the time it caused something to break so I am surprised that works for you.

The _createObjectImpl code keeps references to the objects that are not removed in the destructor
Yes, that should be fixed too, if dispose() is called it should dispose all the owned widgets

If _createChildControlImpl can be used for this then I'm less convinced why we want this mechanism because it would mean that there is already a solution for IDs. The missing piece would be a starting point to resolve absolute ID paths from.

I'll experiment with my mixin to merge it with _createChildControlImpl and see if I can reproduce the problems I found before.

Member

johnspackman commented Nov 22, 2017

Hmm. I don't understand how your example works. Mustn't it at least call the base _createObjectImpl to make the objects beeing registered?

It should, you're right - but not to make the objects become registered (calling base would only be necessary if the base class might create widgets on demand also). When you need to access an object, you call getObject("edtPostcode") and then will only call _createObjectImpl the first time, to create the object - after that it is cached. This is the same pattern as getChildControl/_createChildControlImpl

More thoughts: That mechanism isn't valid if you're using _createChildControlImpl most of the time to control widget styling. Setting up a form is just a small part of an application, and we even use _createChildControlImpl for that here
Fair point, my mixin here will defer to getChildControl if _createObjectImpl returned undefined.

Why not just using the _createChildControlImpl mechanism for that, too? It's feels much more natural if you've large widget structures IMHO.

That was my first attempt years ago but found that there were conflicts with getting the appearances to work, and it might require that everything returned is a LayoutItem or Widget. It's a long time ago now and maybe things have moved on, but at the time it caused something to break so I am surprised that works for you.

The _createObjectImpl code keeps references to the objects that are not removed in the destructor
Yes, that should be fixed too, if dispose() is called it should dispose all the owned widgets

If _createChildControlImpl can be used for this then I'm less convinced why we want this mechanism because it would mean that there is already a solution for IDs. The missing piece would be a starting point to resolve absolute ID paths from.

I'll experiment with my mixin to merge it with _createChildControlImpl and see if I can reproduce the problems I found before.

johnspackman added a commit to johnspackman/qooxdoo that referenced this issue Nov 28, 2017

Merge branch 'object-ids-2'
* object-ids-2:
  lint & better comments
  Provides Universal Widget ID implementation discussed in qooxdoo#9422

johnspackman added a commit to johnspackman/qooxdoo that referenced this issue Jan 23, 2018

Merge branch 'master' of github.com:johnspackman/qooxdoo
* 'master' of github.com:johnspackman/qooxdoo:
  add support for async detection events
  suppress debug output
  fix minor bug
  Fixes a bug where slider animation will recurse into itself, cancelling the animation and restarting it, resulting in an incomplete animation when scrolling.
  more fixes for event promises
  Remove DOM in non-debug builds, fix comment
  rename `discardOwnedObject` and allowed `addOwnedObject` to automatically remove from the previous owner
  first pass
  fixes a strict mode compliance bug
  lint & better comments
  Provides Universal Widget ID implementation discussed in qooxdoo#9422
  fixes a strict mode compliance bug
  lint & better comments
  Provides Universal Widget ID implementation discussed in qooxdoo#9422

# Conflicts:
#	documentation/manual/source/conf.py
#	package.json
@cboulanger

This comment has been minimized.

Show comment
Hide comment
@cboulanger

cboulanger Feb 26, 2018

Contributor

Another use case that just came up in my own code:

The backend needs to reference an object in the qooxdoo application which cannot be known in advance, but is determined dynamically at runtime. At the moment, I solve this by evaling code, which is obviously bad. A deterministic object id would allow to safely reference this object and preserve isolation of application components.

@johnspackman, did you have a chance to experiment with child controls? A difference between parent/child components and child controls would be that the child control know nothing about their parent control (or do they?), whereas child components should have a way to look up their parent.

Contributor

cboulanger commented Feb 26, 2018

Another use case that just came up in my own code:

The backend needs to reference an object in the qooxdoo application which cannot be known in advance, but is determined dynamically at runtime. At the moment, I solve this by evaling code, which is obviously bad. A deterministic object id would allow to safely reference this object and preserve isolation of application components.

@johnspackman, did you have a chance to experiment with child controls? A difference between parent/child components and child controls would be that the child control know nothing about their parent control (or do they?), whereas child components should have a way to look up their parent.

@johnspackman

This comment has been minimized.

Show comment
Hide comment
@johnspackman

johnspackman Feb 26, 2018

Member

No not yet - it's still on my list. I'm keen to do it because I think I have such a good set of test cases, it's getting closer to the top of the todo list but not as fast as I'd like :(

My proposed implementation does have a parent, it's called "owner" so you can navigate up and down

Member

johnspackman commented Feb 26, 2018

No not yet - it's still on my list. I'm keen to do it because I think I have such a good set of test cases, it's getting closer to the top of the todo list but not as fast as I'd like :(

My proposed implementation does have a parent, it's called "owner" so you can navigate up and down

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment