Sigil-Ebook / PageEdit

ePub XHTML Visual Editor
GNU General Public License v3.0
248 stars 28 forks source link

Larger icon for the edit mode #10

Closed BeckyDTP closed 4 years ago

BeckyDTP commented 4 years ago

I know this change does not make sense, but I checked it a few weeks ago and forgot to prepare Pull Request.

How to check?

  1. Go to edit mode (pencil icon)
  2. Set the maximum icons in Preferences> Appearance> Main UI
  3. Note that this one and only icon has not grown.

That is why in this unique place we have to load the "big" pencil right away.

It is possible that there is another solution, but I admit that I did not find it. It just works.

dougmassay commented 4 years ago

Actually, it sort of makes sense. All of the other toolbar icons load the 48px versions as default in the main.ui file. Then we extend them with the 22px versions in MainWindow. But the Edit/pencil icon is not loaded by default in the .ui file. The Preview/eyeball one is. Then we switch to the Edit/pencil programmatically later on. So it probably only ever gets the 22px version to work with.

kevinhendricks commented 4 years ago

Would you please change your pull request ever so slightly to simply add (not shrink) the 48 px size for that icon

It seems it is not being properly parsed from the main.ui file for some strange reason.

Something along these lines perhaps:

diff --git a/MainWindow.cpp b/MainWindow.cpp
index 724f934..735d43f 100644
--- a/MainWindow.cpp
+++ b/MainWindow.cpp
@@ -1853,6 +1853,7 @@ void MainWindow::ExtendIconSizes()
     icon = ui.actionMode->icon();
     icon.addFile(QString::fromUtf8(":/icons/mode-preview_22px.png"), QSize(22,22), QIcon::Normal, QIcon::Off);
     icon.addFile(QString::fromUtf8(":/icons/mode-edit_22px.png"), QSize(22,22), QIcon::Normal, QIcon::On);
+    icon.addFile(QString::fromUtf8(":/icons/mode-edit_48px.png"), QSize(48,48), QIcon::Normal, QIcon::On);
     ui.actionMode->setIcon(icon);

Thanks!

KevinH

dougmassay commented 4 years ago

The solution is probably to add BOTH icons in the ExtendIconSizes section. Can you test if that works? If so, you can add the commit to this PR.

kevinhendricks commented 4 years ago

Great minds think alike! ;)

dougmassay commented 4 years ago

Guess so! :D

BeckyDTP commented 4 years ago

I confirm that. Adding 48px icons works.

dougmassay commented 4 years ago

If you push your commit to your fork, this PR should pick it up and we can add it.

dougmassay commented 4 years ago

Looks good to me. But now that I think about it, I think the main Heading-Change ToolButton icon might need the same treatment (the icons for the individual actions are fine). It's not assigned in main.ui, so only the 22px version exists. Do you want to push another commit to this PR since they're related?

In fact, I'm thinking the two similar ToolButtons (CaseChange and HeadingChange) in Sigil, probably need this treatment as well.

BeckyDTP commented 4 years ago

I admit that I thought about it too. It is true that the correct icon is loading, but only AFTER restarting Sigil.

I will check if it works and update this PR and prepare PR for Sigil.

dougmassay commented 4 years ago

Nevermind. I was mistaken. The 48px icons ARE, in fact, correctly assigned to the QToolButtons in main.ui. No need to add them again in ExtendIconSizes.

dougmassay commented 4 years ago

I'm going to merge this request, now.