aacid added a comment.

  In D14904#313604 <https://phabricator.kde.org/D14904#313604>, @sander wrote:
  
  > Thanks for the patch.
  >
  > - I like 'Expand whole section', too.
  > - I don't think additional separators are necessary.
  > - Four times the same icon looks strange to me, but I don't know what the 
alternatives are.
  
  
  Don't have any icon?
  
  > None of this is blocking to me.  @aacid , do you object to me merging this?
  
  The braces are all in the wrong place, and @ngraham didn't seem to happy with 
the wording as far as i understand.

INLINE COMMENTS

> part.cpp:2996
> +                          m_toc.data(), &TOC::expandRecursively);
> +        popup->addAction( 
> QIcon::fromTheme(QStringLiteral("view-list-tree")), i18n("Collapse 
> recursively"),
> +                          m_toc.data(), &TOC::collapseRecursively);

Would "Collapse children" instead of just "Collapse" make more sense? For some 
reason my brain thought this would close up until the root when i read 
"Collapse recursively"

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D14904

To: Lekensteyn, #okular, ngraham
Cc: sander, aacid, ngraham, okular-devel, #okular

Reply via email to