Skip to content
This repository has been archived by the owner on Feb 17, 2021. It is now read-only.

Use compiled templates for rendering internals of bubble #55

Closed
timlindvall opened this issue Feb 11, 2014 · 7 comments
Closed

Use compiled templates for rendering internals of bubble #55

timlindvall opened this issue Feb 11, 2014 · 7 comments
Assignees
Milestone

Comments

@timlindvall
Copy link
Contributor

There are a couple of current enhancements for configuration changes that affect the internals of each Hopscotch bubble (#33 and #39). I'm thinking, instead of creating new configuration options that affect the bubble display (which could lead to a bloated configuration object over time), what if we could provide the option to use a JavaScript template (or other method) to render the internals of each bubble?

How it would work

A user could define a render method in their tour configuration options or register a template via hopscotch.registerTemplate(Function render). Hopscotch would be agnostic to how this template is written or compiled... all it cares about is a method that it can hand an object off to for rendering the bubble's internals. The object itself would contain information relevant to the current step (title, content, stepNum, totalSteps, button info, labels) and would be responsible for returning HTML that can be set as the bubble's innerHTML.

Other contemplations

  • How to handle events? One possibility is to include some means when declaring the template to provide the IDs that Hopscotch should attach events to. Or, we could have Hopscotch pass a DOM reference to the render method and require it to handle attaching the events.
  • Should render() be limited in its scope to just the inside of the bubble, or should it have responsibility for the structure of the whole bubble? The latter might require some updates to positioning calculations.
  • If we go this route, we may want to consider modifying how Hopscotch currently does bubble rendering to use this new pattern instead.

Any thoughts?

@gkoo
Copy link
Contributor

gkoo commented Feb 14, 2014

I like it. I admit, the HopscotchBubble.render function always felt kind of clunky to me.

Handling events should be pretty easy -- you could just do event delegation on the bubble container, and have clients choose specific IDs or classes for the elements whose events Hopscotch needs to listen to. E.g., for the next button, you must assign an ID of hopscotch-next.

Not really sure what you mean by having render be responsible for the structure of the whole bubble. Can you elaborate?

Love the idea!

@timlindvall
Copy link
Contributor Author

Perhaps just some musing over the split of responsibilities between the theme and the framework, which in this case is expressed as whether render() handles just the innerHTML of the bubble (Hopscotch creates the "bubble" for you to render your content into) or actually renders the bubble itself - arrow and all. Of course, if we put too much responsibility on this render() method (or, more broadly, the theme), then it just ends up as a re-implementation of HopscotchBubble. But then... maybe that might be ok? It gives the end user full control over everything involved with visually showing the bubble.

Makes sense to me to have one event listener on the bubble and do event delegation. I'm on the fence with requiring magic IDs for binding actions... this could be configurable, but then what are the compelling cases for configuring those action selectors?

@timlindvall timlindvall self-assigned this Mar 3, 2014
@timlindvall timlindvall added this to the 0.2.0 milestone Mar 3, 2014
@kate2753
Copy link
Contributor

kate2753 commented Mar 7, 2014

This is a great idea. It will definitely give more flexibility to the users.

Will this render function be used to render each step or just scaffolding of the hopscotch bubble? In other words, will content and title of a step still come from tour's steps config JSON, or will they be rendered as part of template?

If it is just scaffolding, I would be in favor of having this render function render the whole bubble (minus content and title), including outer container and arrow.

@timlindvall
Copy link
Contributor Author

Well, as things have evolved... both. I've been working on this on my fork and so far I've broken things down into two templates. The first template renders the scaffolding for the bubble and is only called once during initialization. The second template is for rendering the internal content of the bubble and is called every step of the tour - this second render method receives JSON specifying details about the step.

That said, I'm playing around with the idea of merging the two templates into one. At first, it felt appropriate to break the two components (structure and content) apart given the current code structure, but only having to set one renderer would be more straightforward for end users. The downside of this is that we'll have to reset the references to the arrow element and content element on each rerender.

@kate2753
Copy link
Contributor

I wonder if there is a way we could support both templates (bubbleShellTemplate and bubbleContentTemplate), but make both of them optional? Both templates can take on as much responsibility as user wants them to. If user wants to render full bubble with bubbleContentTemplate they could.

bubbleShellTemplate would be executed once per tour. bubbleContentTemplate would be executed once per step.

This is just an idea to kick around. I haven't thought through it in much detail.

@timlindvall
Copy link
Contributor Author

Maybe, but the idea of the shell template was to create particular DOM elements (hopscotch-content and hopscotch-arrow) that the control can cache references of, so if we had both, they'd both be required (or at least require defaults). That said, I've done some refactoring to allow everything to be rendered by one template, since we don't need those DOM references until render() anyway.

The code I have is getting pretty close to complete, so I should be able to publish a pull request and readme by later this week, once the directory restructuring is complete and I've merged my changes over into the new structure.

@kate2753
Copy link
Contributor

Pull request #70 merged.

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

3 participants