elvisangelaccio requested changes to this revision.
elvisangelaccio added a comment.
This revision now requires changes to proceed.


  In D22444#496089 <https://phabricator.kde.org/D22444#496089>, 
@elvisangelaccio wrote:
  
  > Please use a descriptive commit message: 
https://community.kde.org/Policies/Commit_Policy#Always_add_descriptive_log_messages
  
  
  Still not fixed ;)

INLINE COMMENTS

> dolphinviewactionhandler.cpp:192
>  
> +    QAction* zoomResetAction = m_actionCollection->addAction( 
> QStringLiteral("view_zoom_reset") );
> +    zoomResetAction->setText(i18nc("@action:inmenu View", "Reset Zoom 
> Level"));

Please remove the spaces before/after `QStringLiteral`

> dolphinviewactionhandler.cpp:203
>                               m_actionCollection);
> -    zoomOutAction->setWhatsThis(i18nc("@info:whatsthis zoom in", "This 
> reduces the icon size."));
> -
> +    zoomOutAction->setWhatsThis(i18nc("@info:whatsthis zoom out", "This 
> reduces the icon size."));
> +    

Unrelated change, should go to its own commit. (feel free to push it without 
review...)

> dolphinviewactionhandler.cpp:468-486
> +    ViewSettingsTab::Mode settingsMode;
> +    switch (m_currentView->mode()) {
> +        case DolphinView::IconsView:
> +            settingsMode = ViewSettingsTab::IconsMode;
> +            break;
> +        case DolphinView::DetailsView:
> +            settingsMode = ViewSettingsTab::DetailsMode;

Sorry but I don't like this solution (i.e. creating a settings tab widget 
inside the action handler). Please try the following instead:

1. Create a new method `DolphinView::resetZoomLevel()`.
2. Call `settings->useDefaults(true)` where `settings` is one of 
`IconsModeSettings::self()`, `CompactModeSettings::self()`, or 
`DetailsModeSettings::self()` (depending on the current 
`DolphinView::viewMode()`).
3. Get the default zoom value from `ViewModeSettings::iconSize()`.

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, spoorun, navarromorales, firef, 
andrebarros, skadinna, emmanuelp, mikesomov

Reply via email to