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

feat(cli): allow multiple reverses #488

Merged
merged 2 commits into from
Nov 12, 2024

Conversation

PeculiarProgrammer
Copy link
Contributor

Description

Issue #308 refers to the inability to reverse multiple times. I fixed this by decreasing the slide index by one if reverse was invoked twice consecutively with no next-slides in between.

More specifically, the issue mentions the following:

  1. Reverse to the end of the previous slide
  2. Go directly to the end of the previous slide
  3. Go directly to the beginning of the previous slide

This PR fixes the first. The second option seems to be the same as the first just without the animation, so I don't think an implementation of that is needed. The third option seems to just be the back button without the animation, which I also don't think is necessary.

Fixes Issue

Closes #308

Check List

Check all the applicable boxes:

  • I understand that my contributions needs to pass the checks;
  • If I created new functions / methods, I documented them and add type hints;
  • If I modified already existing code, I updated the documentation accordingly;
  • The title of my pull request is a short description of the requested changes.

Copy link

codecov bot commented Nov 9, 2024

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 80.96%. Comparing base (3dbe12b) to head (bd168f5).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
manim_slides/present/player.py 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #488      +/-   ##
==========================================
- Coverage   81.05%   80.96%   -0.09%     
==========================================
  Files          23       23              
  Lines        1884     1886       +2     
==========================================
  Hits         1527     1527              
- Misses        357      359       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jeertmans
Copy link
Owner

Hi @PeculiarProgrammer, thanks for your contribution! I will review it when I have some free time, probably on Tuesday or later in the week, but that shouldn't take long! :-)

@jeertmans jeertmans added enhancement New feature or request present Related to the main "present" feature labels Nov 10, 2024
@jeertmans jeertmans changed the title Permit Multiple Reverses feat(cli): allow multiple reverses Nov 12, 2024
@jeertmans
Copy link
Owner

I have reviewed and it looks great, thanks! I will include a line in the changelog and merge.

@jeertmans jeertmans merged commit 988011f into jeertmans:main Nov 12, 2024
33 of 38 checks passed
@jeertmans
Copy link
Owner

Thank you for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request present Related to the main "present" feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Add key binding for previous-and-reverse slide
2 participants