simgunz added a comment.
In D15580#554104 <https://phabricator.kde.org/D15580#554104>, @davidhurka wrote: > In D15580#554079 <https://phabricator.kde.org/D15580#554079>, @simgunz wrote: > > > Should I configure the toolbar so that if the user changes to "text alongside icons" the toolbar appears like this? > > (the user can then expand the remaining icon-only buttons one by one by right clicking on them if he needs to visualize the text) > > > I think that will be easier to tell as soon as a user decides to show the text, and complains about it. Seems a good strategy. > --- > > I looked at your code, and I it looks fine, as far as I am common with the Okular code. But I didn’t compile and test yet. > > I am wondering about //my// code. You merged both my pathes (D21755 <https://phabricator.kde.org/D21755> and D21971 <https://phabricator.kde.org/D21971>) into your development branch, right? Maybe you should rebase on D21971 <https://phabricator.kde.org/D21971>, so Phabricator (or gitlab) doesn’t amalganate unrelated changes. I will dedicate sometime to those reviews this week and see if you can finish them and merge them. Then I'll rebase. > Besides that: How (well) does ToggleActionMenu work for you? I see that you don’t use suggestDefaultAction(). But if I got it right, Okular does not remember annotation tool selection across sessions, so it’s useless. I'll report on this soon, I still need to figure out different combinations of the possible configurations of ToggleActionMenu. > This isn't a standard toolbar; it's a toolbar that can be shown and hidden. Once it's shown, it's not obvious how to hide it--especially now that it shows itself after using a quick annotation tool. That'll make it appear, but it won't be obvious how to make it go away again if it doesn't have an integrated close button. Ok > Even though the HIG recommends against it, I think it's fine to change the button style here since we're quite space-constrained. Ok > Expanding spacers were added in 4357ef235ecb8b8b71ca0867d6cfc02acf292fae <https://phabricator.kde.org/R263:4357ef235ecb8b8b71ca0867d6cfc02acf292fae>. Great, that makes things much easier to implement. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D15580 To: simgunz, #okular, #vdg Cc: trickyricky26, simgunz, ltoscano, cfeck, aacid, davidhurka, knambiar, ngraham, tobiasdeiminger, okular-devel, johnzh, andisa, siddharthmanthan, maguirre, fbampaloukas, joaonetto, kezik, tfella, darcyshen
