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

simple substep implemenation #280

Closed
wants to merge 19 commits into from
Closed

Conversation

misterbrownlee
Copy link

Greetings impress.js overlords:

I have made an unobtrusive and clean attempt at providing the basics of a substep feature.

My implementation follows the syntax and behavior modeled by regular steps:

  • denote a substep by adding the css class 'substep' to elements in a step
  • init will prepare the substeps with the additional css class 'future'
  • next/prev will check within the current step for potential substeps
  • navigation through substeps will set 'present', 'active', and 'past' - just like steps
  • the only variation is substeps remain 'active' when past
  • backward navigation removes 'past/active', restores 'future' as needed
  • I've included substep events, following a similar model as regular steps

Because the only presentation-side change is CSS classes, there would be no effect on existing presentations. A new presentation would have to implement substep specific css to utilize the feature. This is outlined in the comments of the example html and css.

I thought it was best to create a separate example, rather than tinker with the existing index.html, as it seems like someone who wants substeps will find the html and figure it out from there.

Substeps are not included as a separate API feature. I felt this was the right thing because it didn't seem necessary to directly navigate to a substep. I also didn't think deep linking a substep made sense, and the sytax for that would have gotten weird, anyway. It did make sense to hook into next/prev. If you're adding substeps, you probably wouldn't want prev/next to skip them, even if you're moving via the API.

I felt an expected default behavior was to have substeps remain active, but that's easily changed. This could also be easily toggled with a 'data-' property.

The TL;DR notes on my commit are:

  • a couple of helper functions for finding and handling substeps
  • a small if/else check in next/prev to decide if a substep is available
  • a separate example html page with it's own CSS.

I know substeps are 'powerpointish', and thus subject to controversy. They also may not meet the impress.js philosophy. My coding style might not be exactly in-line with the main repo. If the philosophical issue can be overcome, I'm happy to revise my code as needed to be acceptable. Just let me know.

Thanks for considering my contribution!

-- Aaron (aka tehfoo)

@misterbrownlee
Copy link
Author

Thought I'd add an easy link to see this working in my pages branch:
http://tehfoo.github.io/impress.js

Thanks @bartaz for the great tool!

@rubik
Copy link

rubik commented Apr 21, 2013

This is exactly what I needed! 👍 Thanks a lot @tehfoo!
Before I had implemented this through the impress:stepenter and impress:stepleave events: substeps were actually real, empty steps which, when entered, triggered a JS function, responsible for showing the substep and hiding it again. Pretty convoluted!

@misterbrownlee
Copy link
Author

Hey @rubik! Glad it's useful to someone! I'll try and keep my fork up on changes so at least if the pull request never merges my version of impress.js won't be too far behind.

@rubik
Copy link

rubik commented Apr 27, 2013

Thanks! In fact now I'm using the file from your fork.

@misterbrownlee
Copy link
Author

I merged a pull request that fixed a bug in my code. I also tried to keep everything else updated with upstream master, which is why the diff on this pull request is getting noisy. If there's an interest in merging this, I'm happy to start a new pull request that's cleaner. The diffs that matter most are js/impress.js, substep.html (the example), and css/substep.css (for the example)

@ghost
Copy link

ghost commented May 31, 2013

thanks @tehfoo , substep is nice.

@piranna
Copy link

piranna commented Jun 3, 2014

Any estimated date whe this will be merged?

@bartaz
Copy link
Member

bartaz commented Jun 4, 2014

Unfortunately I'm not currently maintaining impress.js code regularly because of other priorities.
Especially I'm not able to review and possibly merge such big changes as this one.

So for those interested in using impress.js with additional features such as sub-steps, I suggest to use an unofficial fork that implements this feature - such as the one in this pull request.

@misterbrownlee misterbrownlee mentioned this pull request Oct 25, 2014
@FagnerMartinsBrack
Copy link
Member

FagnerMartinsBrack commented Feb 7, 2016

Hi @misterbrownlee , 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 PRs like this one to see if the OP is still interested in keep it going.

By the way, this one seems duplicate of #214 and #264.

@carljm
Copy link

carljm commented Feb 7, 2016

This looks like pretty much the same thing as #264, just with slightly different naming choices. I think it's a good approach in general, and I don't care which one is merged.

@FagnerMartinsBrack
Copy link
Member

Closing due to the lack of feedback from the OP. If you want to work in the PR again, feel free to comment here to open up the discussion again.

Also, this PR was opened in the dev branch, as from the CONTRIBUTING.md file, it should be opened against the master branch instead.

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

Successfully merging this pull request may close these issues.

7 participants