D22444: Add Reset Zoom Level action inside View menu

This revision was automatically updated to reflect the committed changes. Closed by commit R318:7165ea7afdf1: Add Reset Zoom Level action inside View menu (authored by ngraham). REPOSITORY R318 Dolphin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22444?vs=65468&id=65746 REVISION D

D22444: Add Reset Zoom Level action inside View menu

elvisangelaccio accepted this revision. This revision is now accepted and ready to land. REPOSITORY R318 Dolphin BRANCH arcpatch-D22444 REVISION DETAIL https://phabricator.kde.org/D22444 To: ngraham, elvisangelaccio, shubham, #dolphin Cc: broulik, cfeck, kfm-devel, kde-doc-english, iasens

D22444: Add Reset Zoom Level action inside View menu

ngraham updated this revision to Diff 65468. ngraham added a comment. Fix it for realsies REPOSITORY R318 Dolphin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22444?vs=65465&id=65468 BRANCH arcpatch-D22444 REVISION DETAIL https://phabricator.kde.org/D22444 AFFECTED FILES

D22444: Add Reset Zoom Level action inside View menu

ngraham updated this revision to Diff 65465. ngraham added a comment. Fix action not getting added to menu REPOSITORY R318 Dolphin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22444?vs=65136&id=65465 BRANCH arcpatch-D22444 REVISION DETAIL https://phabricator.kde.org/D22444

D22444: Add Reset Zoom Level action inside View menu

ngraham updated this revision to Diff 65136. ngraham added a comment. Rebase REPOSITORY R318 Dolphin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22444?vs=64868&id=65136 BRANCH arcpatch-D22444 REVISION DETAIL https://phabricator.kde.org/D22444 AFFECTED FILES doc/index.d

D22444: Add Reset Zoom Level action inside View menu

ngraham added a comment. That's strange, it worked before (see the screenshot), but not I don't see it there either. :/ Will investigate and fix. REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D22444 To: ngraham, elvisangelaccio, shubham, #dolphin Cc: broulik, cfeck

D22444: Add Reset Zoom Level action inside View menu

elvisangelaccio added a comment. Am I wrong or the new action doesn't show up in the View menu on the menubar? REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D22444 To: ngraham, elvisangelaccio, shubham, #dolphin Cc: broulik, cfeck, kfm-devel, kde-doc-english, vmarin

D22444: Add Reset Zoom Level action inside View menu

ngraham marked 3 inline comments as done. REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D22444 To: ngraham, elvisangelaccio, shubham, #dolphin Cc: broulik, cfeck, kfm-devel, kde-doc-english, aprcela, vmarinescu, fprice, gennad, MrPepe, fbampaloukas, alexde, Codezela, f

D22444: Add Reset Zoom Level action inside View menu

ngraham updated this revision to Diff 64868. ngraham added a comment. - Rebase - Address review comments REPOSITORY R318 Dolphin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22444?vs=63767&id=64868 BRANCH arcpatch-D22444 REVISION DETAIL https://phabricator.kde.org/D22444

D22444: Add Reset Zoom Level action inside View menu

broulik added inline comments. INLINE COMMENTS > dolphinui.rc:2 > > - > + > Careful, this clashes with the recent toolbar shuffling > dolphinview.cpp:1351 > +default:mode = ViewModeSettings::IconsMode; > +Q_ASSERT(false); > +

D22444: Add Reset Zoom Level action inside View menu

ngraham added a comment. @elvisangelaccio Ping. REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D22444 To: ngraham, elvisangelaccio, shubham, #dolphin Cc: cfeck, kfm-devel, kde-doc-english, aprcela, vmarinescu, fprice, gennad, MrPepe, fbampaloukas, alexde, Codezela,

D22444: Add Reset Zoom Level action inside View menu

ngraham added a comment. OK, it's ready again now. :) REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D22444 To: ngraham, elvisangelaccio, shubham, #dolphin Cc: cfeck, kfm-devel, kde-doc-english, aprcela, vmarinescu, fprice, gennad, MrPepe, fbampaloukas, alexde, feve

D22444: Add Reset Zoom Level action inside View menu

ngraham updated this revision to Diff 63767. ngraham added a comment. Clean up remaining whitespace issues REPOSITORY R318 Dolphin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22444?vs=63766&id=63767 BRANCH arcpatch-D22444 REVISION DETAIL https://phabricator.kde.org/D22444

D22444: Add Reset Zoom Level action inside View menu

ngraham updated this revision to Diff 63766. ngraham added a comment. Rebase REPOSITORY R318 Dolphin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22444?vs=63765&id=63766 BRANCH arcpatch-D22444 REVISION DETAIL https://phabricator.kde.org/D22444 AFFECTED FILES doc/index.d

D22444: Add Reset Zoom Level action inside View menu

ngraham updated this revision to Diff 63765. ngraham added a comment. Undo unnecessary CMake changes REPOSITORY R318 Dolphin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22444?vs=63764&id=63765 BRANCH arcpatch-D22444 REVISION DETAIL https://phabricator.kde.org/D22444 AFFE

D22444: Add Reset Zoom Level action inside View menu

ngraham updated this revision to Diff 63764. ngraham marked 3 inline comments as done and 2 inline comments as done. ngraham added a comment. Use a more typical approach REPOSITORY R318 Dolphin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22444?vs=62548&id=63764 BRANCH arcpat

D22444: Add Reset Zoom Level action inside View menu

elvisangelaccio added a comment. One more thing: the new action doesn't show up in the View menu from the menubar. REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D22444 To: ngraham, elvisangelaccio, shubham, #dolphin Cc: cfeck, kfm-devel, kde-doc-english, aprcela, v

D22444: Add Reset Zoom Level action inside View menu

elvisangelaccio requested changes to this revision. elvisangelaccio added a comment. This revision now requires changes to proceed. In D22444#496089 , @elvisangelaccio wrote: > Please use a descriptive commit message: https://community.kde.org/P

D22444: Add Reset Zoom Level action inside View menu

ngraham added a comment. @elvisangelaccio ping. REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D22444 To: ngraham, elvisangelaccio, shubham, #dolphin Cc: cfeck, kfm-devel, kde-doc-english, aprcela, vmarinescu, fprice, gennad, fbampaloukas, alexde, feverfew, meven, s

D22444: Add Reset Zoom Level action inside View menu

shubham added a comment. Oops, I had a perception that you have to press - twice to zoom out. Maybe it came to me from Windows. REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D22444 To: ngraham, elvisangelaccio, shubham, #dolphin Cc: cfeck, kfm-devel, kde-doc-englis

D22444: Add Reset Zoom Level action inside View menu

ngraham added a comment. You're not blind, but that's how keyboard shortcuts look in KDE software. F7096840: Screenshot_20190725_114012.png REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D22444 To: ngraham, elvisangelaccio

D22444: Add Reset Zoom Level action inside View menu

shubham added a comment. If I am not blind, it shows ctrl+-. :) REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D22444 To: ngraham, elvisangelaccio, shubham, #dolphin Cc: cfeck, kfm-devel, kde-doc-english, aprcela, fprice, gennad, fbampaloukas, alexde, feverfew, meve

D22444: Add Reset Zoom Level action inside View menu

ngraham added a comment. In D22444#502241 , @shubham wrote: > Shouldn't the zoom out shortcut be ctrl-- or is it already occupied? Um, it already is. :) The screenshot even shows it. :) REPOSITORY R318 Dolphin REVISION DETAIL https:/

D22444: Add Reset Zoom Level action inside View menu

shubham added a comment. Shouldn't the zoom out shortcut be ctrl-- or is it already occupied? REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D22444 To: ngraham, elvisangelaccio, shubham, #dolphin Cc: cfeck, kfm-devel, kde-doc-english, aprcela, fprice, gennad, fbampal

D22444: Add Reset Zoom Level action inside View menu

ngraham added a comment. The HIG is silent on the matter, but Okular and Konsole put the reset action in the middle, so I think it makes sense to do the same here. REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D22444 To: ngraham, elvisangelaccio, shubham, #dolphin

D22444: Add Reset Zoom Level action inside View menu

cfeck added a comment. Does our HIG say the order is In/Out/Reset or In/Reset/Out? REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D22444 To: ngraham, elvisangelaccio, shubham, #dolphin Cc: cfeck, kfm-devel, kde-doc-english, aprcela, fprice, gennad, fbampaloukas, ale

D22444: Add Reset Zoom Level action inside View menu

ngraham edited the summary of this revision. ngraham edited the test plan for this revision. REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D22444 To: ngraham, elvisangelaccio, shubham, #dolphin Cc: kfm-devel, kde-doc-english, aprcela, fprice, gennad, fbampaloukas, alexd

D22444: Add Reset Zoom Level action inside View menu

ngraham added a comment. @elvisangelaccio This is now ready for review. REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D22444 To: ngraham, elvisangelaccio, shubham, #dolphin Cc: kfm-devel, kde-doc-english, aprcela, fprice, gennad, fbampaloukas, alexde, feverfew, mev

D22444: Add Reset Zoom Level action inside View menu

ngraham added a reviewer: Dolphin. ngraham marked 2 inline comments as done. REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D22444 To: ngraham, elvisangelaccio, shubham, #dolphin Cc: kfm-devel, kde-doc-english, aprcela, fprice, gennad, fbampaloukas, alexde, feverfew, me

D22444: Add Reset Zoom Level action inside View menu

ngraham updated this revision to Diff 62548. ngraham added a comment. Add new action to View menu properly REPOSITORY R318 Dolphin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22444?vs=61839&id=62548 BRANCH arcpatch-D22444 REVISION DETAIL https://phabricator.kde.org/D22444

D22444: Add Reset Zoom Level action inside View menu

ngraham commandeered this revision. ngraham edited reviewers, added: shubham; removed: ngraham. ngraham added a comment. Sure, I'd be happy to. REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D22444 To: ngraham, elvisangelaccio, shubham Cc: kfm-devel, kde-doc-english,

D22444: Add Reset Zoom Level action inside View menu

shubham added a comment. In D22444#501560 , @ngraham wrote: > @shubham are you able to make those changes? I am busy right now with GSoC, if you want you can complete it. REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kd

D22444: Add Reset Zoom Level action inside View menu

ngraham added a comment. @shubham are you able to make those changes? REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D22444 To: shubham, ngraham, elvisangelaccio Cc: kfm-devel, kde-doc-english, aprcela, fprice, gennad, fbampaloukas, alexde, feverfew, meven, spoorun,

D22444: Add Reset Zoom Level action inside View menu

ngraham added a comment. Ah, looks like Dolphin conditionally adds the zoom actions to the main menu in `dolphinmainwindow.cpp` (lines 984-988). That's where you should add the new action. Then I guess you don't need to touch the rc files. REPOSITORY R318 Dolphin REVISION DETAIL https:/

D22444: Add Reset Zoom Level action inside View menu

shubham added a comment. I have added the action in .rc files still I can not see them added in the menu. REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D22444 To: shubham, ngraham, elvisangelaccio Cc: kfm-devel, kde-doc-english, aprcela, fprice, gennad, fbampalouka

D22444: Add Reset Zoom Level action inside View menu

shubham retitled this revision from "Add zoom reset action" to "Add Reset Zoom Level action inside View menu". shubham edited the test plan for this revision. REPOSITORY R318 Dolphin REVISION DETAIL https://phabricator.kde.org/D22444 To: shubham, ngraham, elvisangelaccio Cc: kfm-devel, kde-