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 two bugs related to the layout #325

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

tadd
Copy link

@tadd tadd commented Sep 30, 2024

Hi, thank you for maintaining this helpful tool 😃 Pympress has been one of my favorite tools since I started researching as a CS graduate student a few years ago.

I made this PR with my two commits that fix two bugs each other in master. Sorry to include two fixes in one PR, but they are tiny and interrelated, so it'll be better to pack them.

1. Layout changes not saved

As far as I confirmed, the current Pympress does not save and restore any layout changes.

How to reproduce

  1. Launch it.
  2. Make any layout change, e.g., move the center vertical bar almost to the left edge.
  3. Exit and relaunch.
  4. The center vertical bar backs to the original position, not the left edge.

After applying this PR, the vertical bar position is restored as the same as 2.

2. Cannot open "Edit layout" dialog

Pympress fails to open "Edit layout" dialog in the "Starting Configuration" menu with an error message in the console.

How to reproduce

  1. Launch it.
  2. Show "Deck overview" (even if no file opened)
  3. Click "Starting Configuration" -> "Edit layout"

After the 3, I got a message:

Traceback (most recent call last):
  File ".../.local/pipx/venvs/pympress/lib/python3.11/site-packages/pympress/dialog.py", line 463, in show_editor
    self.load_layout()
  File ".../.local/pipx/venvs/pympress/lib/python3.11/site-packages/pympress/dialog.py", line 259, in load_layout
    self.layout_description.set_text(self.layout_descriptions[self.current_layout])
                                     ~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^
KeyError: 'deck-overview'

I can also reproduce after the deck is closed and opened twice or more.

After this PR, the edit dialog opens without the error message.

@tadd
Copy link
Author

tadd commented Oct 24, 2024

Footnote: I can split this up if you need, of course.

@Cimbali
Copy link
Owner

Cimbali commented Oct 24, 2024

Thanks! No worries it looks good and no need to split, I just need to find time to review and merge this properly.

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.

2 participants