Clarify/change builtInButtons

[RobbieTheWagner]

We still call this builtInButtons, but it actually supports custom buttons too now, so we should update docs and possibly change the name.

[BrianSipple]

Recapping our recent discussions, the decision here appears to be just doing away with builtInButtons, and having users use Shepherd's buttons option directly.

[RobbieTheWagner]

@BrianSipple @chuckcarpenter taking a quick look at the builtInButtons code, I think it gives us a few things. One, it binds the action to the this scope of the Ember service, and two it calls next etc in the Ember service, which also triggers events on the Ember side of things. I think we still need builtInButtons, but they are more like emberButtons. If we can replicate all this functionality without it, that's fine to remove it, but I think it's nice to have.

[BrianSipple]

🤔 I'm definitely on the side of making separate functions for separate responsibilities.

[BrianSipple]

More to that point, I think this could be indicative of a good opportunity to refactor the stepsChange observer:

  // TODO: Figure out how to use a computed instead of an observer here
  /**
   * Create a tour object based on the current configuration
   * @private
   */
  stepsChange: observer('steps', function() {
    this.initialize();
    const steps = get(this, 'steps');

    ...
    ...
  }),

What if, instead of having an observer, we just made steps “private” and had a setSteps method that would be required before starting the tour?

(I mean… how often would users be dynamically swapping new steps in and out? And is that the kind of usage we'd want to promote?)

Such a function could then be our jumping off point for refactoring builtInButtons and isolating the other concerns the observer is attempting to handle.

[RobbieTheWagner]

The idea behind the observer is you can always do this.set('steps', to trigger your new tour steps. It sounds like what you are proposing, where we would have a setSteps method, is essentially the same API to the end user of just setting the steps and it works, so that seems fine to me.

[chuckcarpenter]

I guess my feeling is aren't we trying to emulate the docs for Shepherd and make them work in Ember with some additional integration points?

[chuckcarpenter]

Also, I wonder if we should have the ability to add individual steps along the way, kind of as if a user took another path. This could just be done with this.get('tour.tourObject').addStep() though

[BrianSipple]

I like the idea of setSteps, addStep, and addSteps as separate points of interaction.

[RobbieTheWagner]

@chuckcarpenter

Also, I wonder if we should have the ability to add individual steps along the way, kind of as if a user took another path. This could just be done with this.get('tour.tourObject').addStep() though

You would do this currently by just pushing onto the steps array, and the observer would fire.

@BrianSipple I think supporting all those sounds logical. I want to have an addSteps in Shepherd itself, so you don't have to forEach and addStep.

[chuckcarpenter]

I think this is clearer for the user. Refactoring the addition of steps themselves in another issue. #241