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

[Question] How to populate the div within a Surface using Javascript #59

Closed
Iftahh opened this issue Jul 3, 2016 · 8 comments
Closed

Comments

@Iftahh
Copy link

Iftahh commented Jul 3, 2016

Say I want to insert a CodeMirror IDE inside a surface, or a JQuery UI (eg. jsTree), or an Ember component, etc...

There doesn't seem to be a documented way to insert content into a Surface except for the content string, which is VERY limiting.

Something along getting the DOM element of the Surface and modifying it when the Surface is initialized is a not a very holistic solution but its good enough for many useful use cases.

@dmvaldman
Copy link
Owner

surface.setContent can take a string, a document fragment, or also a DOM element. If you have access to the parent DOM element of a jQuery widget, Ember component etc, then you should be able to inject it into a surface with this method.

A surface's DOM element is also in an (obscure) property at surface._currentTarget. However, overriding this property is not recommended. Upon creating the surface, the DOM element isn't there yet. You'll know when the DOM element exists by listening for the deploy event like so:

    surface.on('deploy', function(domElement){
        // domElement is the DOM element the surface controls the layout of
    });

I'm open to suggestions here if you think there's a better/easier way of doing things.

@dmvaldman
Copy link
Owner

dmvaldman commented Jul 4, 2016

Also, when you first create the surface you can do something like

var surface = new Surface({
    content : document.querySelector("#myDiv"),
    ....
})

@Iftahh
Copy link
Author

Iftahh commented Jul 4, 2016

Thank you for your quick response!

Oops, I just noticed I submitted my question incomplete, even mid-sentence... its very late here, I'm sleepy... - fixed now

The solutions you described ( the other options for 'content', on('deploy') event, and _currentTarget) should work just fine for my needs.

I imagine it will be useful for other developers as well - I suggest to add documentations about it, and maybe make a new example with surfaces that contain some rich UI widgets (eg. datetime picker, RGB picker, etc...).

For a more robust integration I suggest defining a lifecycle of Surface insert/rerender/removal - with events or methods that can be overridden -
eg. Maybe something along (I'm not sure these events make sense in Samsara - still learning...) -
before-render, render, after render, before-insert, after-insert, before-remove, after-remove

A thought about re-rendering and animations - since the inner widget rendering may be very slow, re-rendering the content while (for example) a size changes can lead to crappy resize animation - maybe just re-render when the animation completes.

@dmvaldman
Copy link
Owner

To your last point, I think we can do something to defer the setContent result until an animation completes. Maybe this could be an (optional) second argument to the method.

Currently there are two life-cycle events that surfaces listen to: deploy and recall (which happens when the surface is removed). I'm not exactly sure of the need for the "before" and "after" options, such as "before deploy", "after deploy", etc. But maybe there is a good reason to have them? Have you needed to distinguish between them?

@Iftahh
Copy link
Author

Iftahh commented Jul 4, 2016

Hmm, I agree "after deploy" is much more useful than "before deploy" -
similarly "before recall" is more useful than "after recall"

So I agree no real need to add the less useful events.

I can imagine use case where the "before deploy" can be useful - especially if the framework waits for the "before deploy" to complete asynchronously (eg. by having the "before" method to return a promise and continuing the deploy after the promise succeeds)

For example suppose you want to load data (eg. from AJAX, IndexedDB, etc..) just prior to showing a Surface, wherever it is located, however it is added.
You don't want to load the data AFTER the DOM has been inserted, since that would flash to the screen a Surface without any data followed by a re-render of the loaded results - its much better to delay adding the Surface until the data is loaded.
You can load the data just prior to adding the Surface, but that passes the responsibility to the containing unit (maybe this is a good thing - I don't think UI components should call AJAX internally... but this is a matter of opinion)

@jd-carroll
Copy link
Contributor

I would be very hesitant to introduce any type of "exotic" event triggers such as before deploy, and would personally vote 👎.

If you need to load a library, either load the library at bootstrap or use the mount trigger. If you need to load data, set the opacity to 0. And I would disagree with your assertion that you wouldn't want a "flash". Having a true flash is never good, but appropriately managing the display and showing pending content while loading isnt a bad thing. Also, since this data is contained within the Surface, the render tree wouldn't be touched when the data is finally loaded (so no re-render).

So I'm definitely 👎 for expanding the current deploy and recall events.

@Iftahh
Copy link
Author

Iftahh commented Jul 5, 2016

I'm fine with leaving the current deploy and recall events, I wouldn't have raised this question if I had known about them and I only suggested the extra events as quick suggestion off the top of my head.

But -

  1. adding extra hooks or methods that can overridden has very little downside - they are optional opt-in and if you do not use them then nothing changes. If you leave them out of the tutorials they have no impact on the learning curve.
  2. If you see a common use case that many developers will encounter, and the framework can make that use case handling easier, and it has no real downside (ie. does not require refactoring the entire framework, does not add code complexity, does not make the learning curve harder, etc...) then I wouldn't rule it out.
  3. Its possible to add the View with opacity set to 0 and make it visible after the data is loaded and real content is deployed - but this is a common use case that many developers will encounter - and I personally prefer NOT adding to the DOM at all until it is ready (ie. if the data load fails I would rather not handle removing the invisible component).
  4. Splitting the the lifecycle into "initialize before added to the DOM" (useful for loading state) and "initialize after added to the DOM" (useful for adding event listeners, running jquery plugins, etc.) makes sense to me as a Samsara framework responsibility.
    (*) For the "initialize before added to the DOM" to be useful it has to delay the "deploy" - so can't be an event.

@dmvaldman
Copy link
Owner

Good points all around. It should be known that deploy and recall events are similar but not the same as, say, the life-cycle events in React. A recall event may happen when a surface is removed from the scrollview, for instance, because it has been scrolled far out of view, and deployed again when scrolled back into view. The user of Samsara may not know when this happens, and shouldn't need to. As a result, they shouldn't need to manage anything. Event listeners, for example, don't need to be attached or removed (instead they are stored in memory when a recall happens). I never understood why React didn't just do this, and instead made a bunch of life-cycle events and punted the responsibility to the user.

For now I'd like to keep the life-cycle surface area to a minimum, and we'll see if there are any issues raised as a result. In which case I'm happy to add more hooks into the system.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants