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

Fix default substep order when no explicit order given (#855) #856

Merged
merged 1 commit into from
Dec 5, 2023

Conversation

treitmayr
Copy link
Contributor

This PR fixes the default order of substeps, for which no explicit order was given (see issue #855). It ensures that the loop for marking substeps "active" + "visible" is left when at least one substep (with explicit or default order) was marked and this substep uses default order.

Note: The removed space in the header of js/impress.js does not relate to the current issue, but comes from regenerating this file. Apparently the file has not been regenerated when its source file was modified in some previous commit.

This affects both going to the next and previous substep.
@treitmayr
Copy link
Contributor Author

I had to update the PR because I realized that not only going to the next substep was broken, but also going to the previous one. Should work now.

@fnogatz
Copy link
Contributor

fnogatz commented Mar 19, 2023

Thank you, @treitmayr, for your contribution! Indeed, this was a bug in the substep plugin. I have not looked in detail into its history, but maybe it was introduced by my modifications in #825 🙈

I tested your modifications and they work as expected, thank you! @henrikingo, this is good to merge :)

Copy link
Contributor

@janishutz janishutz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as it should, ready to merge

@henrikingo henrikingo merged commit b3a7a63 into impress:master Dec 5, 2023
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.

None yet

4 participants