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

make menu entries Show embedded PDF large/small consistent #3931

Merged
merged 3 commits into from
Jan 5, 2025

Conversation

octaeder
Copy link
Contributor

@octaeder octaeder commented Jan 3, 2025

This PR adresses following two menu entries:

grafik

Both actions are always enabled even when there is no pdf-viewer at all or the viewer is not embedded. When the viewer is emedded exactly one of the entries should be enabled. So we have three cases:

  • no (embedded) viewer:
    grafik
  • small embedded viewer:
    grafik
  • large embedded viewer
    grafik

newManagedAction(menu, "enlargePDF", tr("Show embedded PDF large"), SLOT(enlargeEmbeddedPDFViewer()));
newManagedAction(menu, "shrinkPDF", tr("Show embedded PDF small"), SLOT(shrinkEmbeddedPDFViewer()));

newManagedAction(menu, "enlargePDF", tr("Show embedded PDF large"), SLOT(enlargeEmbeddedPDFViewer()))->setVisible(false);
Copy link
Member

Choose a reason for hiding this comment

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

please make two lines out of this, i.e
act=...
act->setVi...

Copy link
Member

Choose a reason for hiding this comment

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

and the approach seems to be a bit indirect.
You want to change menu entries in texstudio.cpp level, the changes are triggered there (even embedding/windowing over Texstudio::runInternalPdfViewer)

newManagedAction(menu, "shrinkPDF", tr("Show embedded PDF small"), SLOT(shrinkEmbeddedPDFViewer()));

newManagedAction(menu, "enlargePDF", tr("Show embedded PDF large"), SLOT(enlargeEmbeddedPDFViewer()))->setVisible(false);
newManagedAction(menu, "shrinkPDF", tr("Show embedded PDF small"), SLOT(shrinkEmbeddedPDFViewer()))->setVisible(false);
Copy link
Member

Choose a reason for hiding this comment

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

please make two lines out of this, i.e
act=...
act->setVi...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just in case that there was no automatic notification, I want to note that I changed the two lines.

@sunderme
Copy link
Member

sunderme commented Jan 3, 2025

Did you see my other comment ?
and the approach seems to be a bit indirect. You want to change menu entries in texstudio.cpp level, the changes are triggered there (even embedding/windowing over Texstudio::runInternalPdfViewer)

@octaeder
Copy link
Contributor Author

octaeder commented Jan 3, 2025

There are two pairs of similar actions, one of them connected to a button of PDFdocument. So it seems best to me to use the ui changes to setup the menu and the button.

@sunderme
Copy link
Member

sunderme commented Jan 4, 2025

I have looked through the code.
pdfdocument emits triggeredShrink/triggeredEnlarged which are then handled in TexStudio::

Please rewrite the code that the menu change is done in the TexStudio class only.

Secondly, the menu should be disabled, not hidden. The menu entries are always a good learning source for shortcuts.

@octaeder
Copy link
Contributor Author

octaeder commented Jan 5, 2025

Thanks for the hints. I made a new commit.

@octaeder
Copy link
Contributor Author

octaeder commented Jan 5, 2025

and updatet first commit (images etc.) of th PR.

@sunderme sunderme merged commit 66b810b into texstudio-org:master Jan 5, 2025
7 checks passed
@sunderme
Copy link
Member

sunderme commented Jan 5, 2025

thanks

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