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

Add innerStep support. #264

Closed
wants to merge 1 commit into from
Closed

Add innerStep support. #264

wants to merge 1 commit into from

Conversation

carljm
Copy link

@carljm carljm commented Mar 1, 2013

This is my simplified version of #214, leaving all of the styling of inner steps to CSS.

@FagnerMartinsBrack
Copy link
Member

FagnerMartinsBrack commented Feb 5, 2016

Hi @carljm, in an effort to clear up older issues/PRs we are pinging back to know if you are still tracking this request.

To give a little bit of context, recently a decision was made in the project to make the development more active and the first task is to clear up older issues like this one to see if the OP is still interested in keep it going.

By the way, this seems duplicate of the substep functionality proposed on #207

@carljm
Copy link
Author

carljm commented Feb 5, 2016

Yes, I think this is roughly the same functionality as #207. This version of the functionality worked well for me in practice and was easy to use; I'd need to take a closer look at #207 to see how it compares in practice.

@FagnerMartinsBrack
Copy link
Member

Do you want to take a look and check if the functionality is the same or similar? Eventually we need to close one of them because it doesn't make sense having 2 duplicated Pull Requests =/

@carljm
Copy link
Author

carljm commented Feb 7, 2016

After further review, I think this is rather different from #207 (though they are similar enough that I don't think both should be merged; rather one of them should be improved if needed to make sure it can handle the same use cases as the other).

AFAICT #207 is usable only by a JS developer. The substeps are not elements in the markup that one steps through, they are callback functions attached to a slide via somewhat circuitous means (you have a global window.impressSubsteps object, which is a string-indexed collection of callback functions, and then you reference one of those callback functions with a string in the slide's data-substep attribute). The callback can do anything it wants on an attempt to advance to the next slide, and it can prevent the slide from advancing by returning false. This is a very flexible system, but on the downside it has a much steeper barrier to entry, especially for non-developers.

With this PR, you can declare any element in your slide an "innerStep" by just giving it the CSS class .innerstep. As you advance, each innerstep within the slide, from first to last, is given the CSS class .stepped. Thus it's trivial with only CSS knowledge to do a typical set of "revealed" substeps, by just styling .innerstep as invisible and .innerstep.stepped as visible. For more advanced use cases requiring custom JS, this PR also offers the "innerstep" event, which allows running arbitrary JS at the time of an innerstep advance.

So I believe that this PR ultimately offers the same flexibility as #207 for when its needed, but makes the simple cases much simpler (and usable by non-developers). Also, I think it's more consistent with the existing impress.js API, in that the hook for custom code is an impress event, not the introduction of an entirely new global object full of callback functions.

@carljm
Copy link
Author

carljm commented Feb 7, 2016

(I wouldn't have any problem with updating this PR to rename them from "innerSteps" to "substeps", if that naming is preferred. Or any other changes that are desired.)

@FagnerMartinsBrack
Copy link
Member

@carljm Thanks for the feedback! We will definitely revisit this PR later. If you want to keep track of what we are doing, see #512

@henrikingo
Copy link
Contributor

See substep plugin, now merged to master.

@henrikingo henrikingo closed this Dec 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants